Closed Bug 776802 Opened 7 years ago Closed 7 years ago

[Azure] gfxPlatform should be more precise in providing info about Azure backends

Categories

(Core :: Graphics, defect)

15 Branch
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla17

People

(Reporter: nrc, Assigned: nrc)

References

Details

Attachments

(1 file, 1 obsolete file)

gfxPlatform::SupportsAzure should not return information about Azure backends. gfxPlatform::GetAzureBackendInfo should be removed. If needed we should add API for returning info about the preferred and fallback backends for canvas and content. But, in general any info should be taken from an actual DrawTarget, or there shouls be gfxPlatform methods to create a DrawTarget.

The theory here is that we can support multiple different Azure backends at one time, so the backend which will be used in some situation is not necessarily the one returned by the current gfxPlatform methods, so this info is misleading and possibly dangerous.

about:config should be changed to reflect the more complex nature of Azure backends, i.e., there should not be a single entry for 'Azure backend' but separate ones for canvas and content and canvas fallback.
Should probably check the various places that query the Azure prefs either directly (hopefully none) or via gfxInfo to make sure they are not assuming a single type of Azure backend.
Blocks: 778595
Component: Graphics → GFX: Color Management
Assignee: nobody → ncameron
Component: GFX: Color Management → Graphics
In the description, s/about:config/about:support
Attached patch patch (obsolete) — Splinter Review
Attachment #647051 - Flags: review?(bas.schouten)
Comment on attachment 647051 [details] [diff] [review]
patch

Review of attachment 647051 [details] [diff] [review]:
-----------------------------------------------------------------

::: gfx/thebes/gfxWindowsPlatform.h
@@ +243,5 @@
>  
>  protected:
> +    virtual mozilla::gfx::BackendType GetContentBackend()
> +    {
> +      return UseAzureContentDrawing() && mD2DDevice ?

Probably better to check for D3D10 layers here.
Attachment #647051 - Flags: review?(bas.schouten) → review+
(In reply to Bas Schouten (:bas) from comment #4)
> Comment on attachment 647051 [details] [diff] [review]
> patch
> 
> Review of attachment 647051 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: gfx/thebes/gfxWindowsPlatform.h
> @@ +243,5 @@
> >  
> >  protected:
> > +    virtual mozilla::gfx::BackendType GetContentBackend()
> > +    {
> > +      return UseAzureContentDrawing() && mD2DDevice ?
> 
> Probably better to check for D3D10 layers here.

Do you mean instead of checking mD2DDevice?
(In reply to Bas Schouten (:bas) from comment #4)
> Comment on attachment 647051 [details] [diff] [review]
> patch
> 
> Review of attachment 647051 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: gfx/thebes/gfxWindowsPlatform.h
> @@ +243,5 @@
> >  
> >  protected:
> > +    virtual mozilla::gfx::BackendType GetContentBackend()
> > +    {
> > +      return UseAzureContentDrawing() && mD2DDevice ?
> 
> Probably better to check for D3D10 layers here.

I'm not sure what this means - we might be using different layer managers in different places, but this is a function on a global gfxPlatform object, so I can't check which layer manager is being used, the best option is to take a guess at which backend will be used in general, and that will probably be D2D, if we can use D2D and it is pref'ed on, right?
Attached patch patchSplinter Review
changed d2d device check to use RenderMode, carrying r=bas

discussed Bas's comment on and this change on irc: no good way to check for D3D10 layers, don't need to be 100% precise here because info is only used for tests
Attachment #647051 - Attachment is obsolete: true
Attachment #647325 - Flags: review+
Unfortunately the bug number in the backout typoed, similar to bug 718849 comment 22 (was listed as bug 776803). I've commented in that bug pointing here, in case people come across it via hg blame.
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla17
The landing was manually deselected in m-cMerge, but it ended up being marked still (will file a bug on m-cMerge). Reopening since was backed out.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Target Milestone: mozilla17 → ---
https://hg.mozilla.org/mozilla-central/rev/4976c4bb8b27
Status: REOPENED → RESOLVED
Closed: 7 years ago7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla17
You need to log in before you can comment on or make changes to this bug.