WebGL - implement powerPreference

NEW
Assigned to

Status

()

Core
Canvas: WebGL
P2
normal
10 months ago
6 days ago

People

(Reporter: daoshengmu, Assigned: daoshengmu)

Tracking

(Depends on: 1 bug, Blocks: 1 bug, {dev-doc-needed, feature})

unspecified
Future
dev-doc-needed, feature
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [gfx-noted], URL)

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(4 attachments, 1 obsolete attachment)

(Assignee)

Description

10 months ago
Add powerPreference support under Context creation parameters to provides a hint to the user agent indicating what configuration of GPU is suitable for this WebGL context.

See https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.2.1
(Assignee)

Updated

10 months ago
Summary: WebGL powerPreference support → WebGL - implement powerPreference
Keywords: feature
Priority: -- → P3
Whiteboard: [gfx-noted]
(Assignee)

Updated

10 months ago
Blocks: 1207170
Keywords: dev-doc-needed
(Assignee)

Updated

2 months ago
See Also: → bug 1339215
Let's aim for ff60.
Assignee: nobody → dmu
Priority: P3 → P2
Target Milestone: --- → Future
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Assignee)

Comment 5

15 days ago
According to Virtual display [1], it mentions OSX will create several virtual displays for all available GPU cards and a software renderer. Therefore, in the case of MBP 15" 2015, it has three virtual display (0: integrated gpu, 1: AMD gpu, 3: software). We just need to change the virtual display id, then we can make it switch to different OpenGL context from different gpu cards.


[1] https://developer.apple.com/library/content/documentation/GraphicsImaging/Conceptual/OpenGL-MacProgGuide/opengl_pg_concepts/opengl_pg_concepts.html
(Assignee)

Comment 6

13 days ago
Created attachment 8941344 [details]
The test case for WebGL power preference
(Assignee)

Comment 7

13 days ago
(In reply to Daosheng Mu[:daoshengmu] from comment #6)
> Created attachment 8941344 [details]
> The test case for WebGL power preference

I run the test case in Safari and Chromium, but it has no effect when changing to other power preference modes. I suppose the WebGL renderer would be changed to the integrated GPU instead of the discrete GPU when my laptop's battery is less than 35%.
(Assignee)

Comment 8

13 days ago
Created attachment 8941348 [details]
The test case for WebGL power preference

update for recreating canvas element and gl context when changing the powerPreference mode.
Attachment #8941344 - Attachment is obsolete: true
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Assignee)

Comment 12

13 days ago
Comment on attachment 8940664 [details]
Bug 1349799 - Part 3: WebGL PowerPreference support on OSX;

Jeff, please give me some feedback about the powerpreference implementation on OSX. I try the test case from attachment 8941348 [details] in Safari and Chromium, but both of them don't work. The discussion from Apple [1] is very close what I did currently. It is "low-power" by default, but removing NSOpenGLPFAAllowOfflineRenderers when request for "high-performance".

[1] https://www.khronos.org/webgl/public-mailing-list/public_webgl/1703/msg00023.php
Attachment #8940664 - Flags: feedback?(jgilbert)

Comment 13

13 days ago
mozreview-review
Comment on attachment 8940664 [details]
Bug 1349799 - Part 3: WebGL PowerPreference support on OSX;

https://reviewboard.mozilla.org/r/210914/#review217702

::: gfx/gl/GLContextProviderCGL.mm:322
(Diff revision 2)
> +        memcpy(attribs, kAttribs_offscreen_coreProfile, sizeof(kAttribs_offscreen_coreProfile));
> +
> +        if (powerPreference == PowerPreferenceFlags::HIGH_PERFORMANCE) {
> +            // At high performance mode, we don't want to switch to
> +            // offline renderer, (NSOpenGLPFAAllowOfflineRenderers).
> +            attribs[3] = (NSOpenGLPixelFormatAttribute)0;

Use a different array for this, or dynamically generate using a vector.

Generally we want to branch to add something, not to remove something.

::: gfx/gl/GLContextProviderCGL.mm:326
(Diff revision 2)
> +            // offline renderer, (NSOpenGLPFAAllowOfflineRenderers).
> +            attribs[3] = (NSOpenGLPixelFormatAttribute)0;
> +        }
> +#if MAC_OS_X_VERSION_MIN_REQUIRED < MAC_OS_X_VERSION_10_5
> +        // NSOpenGLPFAAllowOfflineRenderers is not supported on
> +        // OS X 10.4 or earlier.

We don't support these old versions, so drop this ifdef.

::: gfx/gl/GLContextProviderCGL.mm:413
(Diff revision 2)
>          triedToCreateContext = true;
>  
>          MOZ_RELEASE_ASSERT(!gGlobalContext);
>          nsCString discardFailureId;
>          RefPtr<GLContext> temp = CreateHeadless(CreateContextFlags::NONE,
> +                                                PowerPreferenceFlags::DEFAULT,

Roll this flag into CreateContextFlags as HIGH_POWER or something.

Comment 14

13 days ago
mozreview-review
Comment on attachment 8939767 [details]
Bug 1349799 - Part 1: Add WebGL PowerPreference attribute;

https://reviewboard.mozilla.org/r/210074/#review217704

::: dom/canvas/WebGLContext.cpp:772
(Diff revision 3)
>  
>      //////
>  
>      if (tryNativeGL) {
>          if (useEGL)
> -            return CreateAndInitGLWith(CreateGLWithEGL, baseCaps, flags, out_failReasons);
> +            return CreateAndInitGLWith(CreateGLWithEGL, baseCaps, flags, mOptions.powerPreference,

CreateAndInitGLWith isn't static, so it has access to mOptions.
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Assignee)

Comment 18

11 days ago
I would like to ask for reviewing the current work for Mac platform. Then, I will keep working on powerPreference support on other platforms.
(Assignee)

Comment 19

8 days ago
For Nvidia Optimus and AMD PowerXpress on Windows, we can refer this discussion. But, I am not sure if it would affect the context that is created from other processes.
(Assignee)

Comment 20

8 days ago
(In reply to Daosheng Mu[:daoshengmu] from comment #19)
> For Nvidia Optimus and AMD PowerXpress on Windows, we can refer this
> discussion. But, I am not sure if it would affect the context that is
> created from other processes.

Adding the discussion link, https://stackoverflow.com/questions/6036292/select-a-graphic-device-in-windows-opengl/27881472#27881472.

Comment 21

7 days ago
mozreview-review
Comment on attachment 8939767 [details]
Bug 1349799 - Part 1: Add WebGL PowerPreference attribute;

https://reviewboard.mozilla.org/r/210074/#review219090

::: dom/canvas/WebGLContext.cpp:594
(Diff revision 4)
>  
>      MOZ_RELEASE_ASSERT(!gl, "GFX: Already have a context.");
>      RefPtr<gl::GLContext> potentialGL;
>      while (!fallbackCaps.empty()) {
>          const gl::SurfaceCaps& caps = fallbackCaps.front();
> -        potentialGL = fnCreateGL(caps, flags, this, out_failReasons);
> +        potentialGL = fnCreateGL(caps, flags, this, mOptions.powerPreference,

Why isn't the power preference passed via `flags`?
Also this looks like it should be in a later patch in this queue.
Attachment #8939767 - Flags: review?(jgilbert) → review+

Comment 22

7 days ago
mozreview-review
Comment on attachment 8940664 [details]
Bug 1349799 - Part 3: WebGL PowerPreference support on OSX;

https://reviewboard.mozilla.org/r/210914/#review219122
Attachment #8940664 - Flags: review?(jgilbert) → review+

Comment 23

7 days ago
mozreview-review
Comment on attachment 8941365 [details]
Bug 1349799 - Part 2: Adding high power context creation flag;

https://reviewboard.mozilla.org/r/211682/#review219124

::: dom/canvas/WebGLContext.cpp:97
(Diff revision 2)
>  using namespace mozilla::dom;
>  using namespace mozilla::gfx;
>  using namespace mozilla::gl;
>  using namespace mozilla::layers;
>  
> +#define POWER_PREFERENCE_EVENT(event, power, flags) ((power == dom::WebGLPowerPreference::High_performance) && \

This is way too much logic for a macro. Just use a function, stored to a clearly named intermediate variable.

::: dom/canvas/WebGLContext.cpp:101
(Diff revision 2)
>  
> +#define POWER_PREFERENCE_EVENT(event, power, flags) ((power == dom::WebGLPowerPreference::High_performance) && \
> +    (((event) & \
> +    (CanvasEventType::WEBGL_CONTEXT_LOST|CanvasEventType::WEBGL_CONTEXT_RESTORED)) == \
> +    (CanvasEventType::WEBGL_CONTEXT_LOST|CanvasEventType::WEBGL_CONTEXT_RESTORED)) ? \
> +    flags | gl::CreateContextFlags::HIGH_POWER : flags)

The default for now should be HIGH_POWER. We will eventually add a heuristic for interpreting "default" as low-power sometimes, but for now, "default" should mean high-power.

::: dom/html/HTMLCanvasElement.cpp:631
(Diff revision 2)
> +                                    EventListener* aCallback,
> +                                    const AddEventListenerOptionsOrBoolean& aOptions,
> +                                    const Nullable<bool>& aWantsUntrusted,
> +                                    ErrorResult& aRv)
> +{
> +  if (aType == NS_LITERAL_STRING("webglcontextlost")) {

This doesn't handle if RemoveEventListener is called afterwards. Ask someone from DOM how this should be done. :dholbert may be able to get you started.

Let's handle this in a follow-up bug, and just ignore the requirement of events for now.
Attachment #8941365 - Flags: review?(jgilbert) → review-
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Assignee)

Updated

6 days ago
Depends on: 1431013
(Assignee)

Comment 27

6 days ago
mozreview-review-reply
Comment on attachment 8939767 [details]
Bug 1349799 - Part 1: Add WebGL PowerPreference attribute;

https://reviewboard.mozilla.org/r/210074/#review219090

> Why isn't the power preference passed via `flags`?
> Also this looks like it should be in a later patch in this queue.

Agree. I have moved this fix to the patch 2.
(Assignee)

Comment 28

6 days ago
mozreview-review-reply
Comment on attachment 8941365 [details]
Bug 1349799 - Part 2: Adding high power context creation flag;

https://reviewboard.mozilla.org/r/211682/#review219124

> This doesn't handle if RemoveEventListener is called afterwards. Ask someone from DOM how this should be done. :dholbert may be able to get you started.
> 
> Let's handle this in a follow-up bug, and just ignore the requirement of events for now.

I file a follow-up Bug 1431013 to handle these events.
(Assignee)

Comment 29

6 days ago
(In reply to Daosheng Mu[:daoshengmu] from comment #20)
> (In reply to Daosheng Mu[:daoshengmu] from comment #19)
> > For Nvidia Optimus and AMD PowerXpress on Windows, we can refer this
> > discussion. But, I am not sure if it would affect the context that is
> > created from other processes.
> 
> Adding the discussion link,
> https://stackoverflow.com/questions/6036292/select-a-graphic-device-in-
> windows-opengl/27881472#27881472.

I have tried to add "NvOptimusEnablement" at GLContextProviderEGL.cpp and GLContextProviderWGL.cpp in Windows, but it still get the context from the integrated GPU. I suppose it should allow us to create the context from the discrete GPU.
(In reply to Daosheng Mu[:daoshengmu] from comment #29)
> (In reply to Daosheng Mu[:daoshengmu] from comment #20)
> > (In reply to Daosheng Mu[:daoshengmu] from comment #19)
> > > For Nvidia Optimus and AMD PowerXpress on Windows, we can refer this
> > > discussion. But, I am not sure if it would affect the context that is
> > > created from other processes.
> > 
> > Adding the discussion link,
> > https://stackoverflow.com/questions/6036292/select-a-graphic-device-in-
> > windows-opengl/27881472#27881472.
> 
> I have tried to add "NvOptimusEnablement" at GLContextProviderEGL.cpp and
> GLContextProviderWGL.cpp in Windows, but it still get the context from the
> integrated GPU. I suppose it should allow us to create the context from the
> discrete GPU.

Let's leave non-mac implementations to follow-up bugs.
You need to log in before you can comment on or make changes to this bug.