Note: There are a few cases of duplicates in user autocompletion which are being worked on.

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

RESOLVED FIXED in mozilla17

Status

()

Core
Graphics
RESOLVED FIXED
5 years ago
3 years ago

People

(Reporter: nrc, Assigned: nrc)

Tracking

15 Branch
mozilla17
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

5 years ago
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.
(Assignee)

Comment 1

5 years ago
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.
(Assignee)

Updated

5 years ago
Blocks: 778595
(Assignee)

Updated

5 years ago
Component: Graphics → GFX: Color Management
(Assignee)

Updated

5 years ago
Assignee: nobody → ncameron
Component: GFX: Color Management → Graphics
(Assignee)

Comment 2

5 years ago
In the description, s/about:config/about:support
(Assignee)

Comment 3

5 years ago
Created attachment 647051 [details] [diff] [review]
patch
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+
(Assignee)

Comment 5

5 years ago
(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?
(Assignee)

Comment 6

5 years ago
(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?
(Assignee)

Comment 7

5 years ago
Created attachment 647325 [details] [diff] [review]
patch

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+
(Assignee)

Comment 8

5 years ago
https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=e4baff472b7f
(Assignee)

Comment 9

5 years ago
backed out: https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=cda1c40da986
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.

Updated

5 years ago
Status: NEW → RESOLVED
Last Resolved: 5 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 → ---
(Assignee)

Comment 12

5 years ago
https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=7198b3ea1763
https://hg.mozilla.org/mozilla-central/rev/4976c4bb8b27
Status: REOPENED → RESOLVED
Last Resolved: 5 years ago5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla17
Duplicate of this bug: 719302
You need to log in before you can comment on or make changes to this bug.