[D2D] Do not push cleartype layers for transparent surfaces

RESOLVED FIXED

Status

()

defect
RESOLVED FIXED
9 years ago
9 years ago

People

(Reporter: bas.schouten, Assigned: bas.schouten)

Tracking

Trunk
x86
Windows 7
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

Assignee

Description

9 years ago
When using a surface with content_type_t COLOR_ALPHA we should not use cleartype. We should also not use it as an option when pushing layers, from the D2D documentation:

http://msdn.microsoft.com/en-us/library/dd368124%28v=VS.85%29.aspx

'ClearType antialiasing must use the current contents of the render target to blend properly. When a pushed layer requests initializing for ClearType, Direct 2D copies the current contents of the render target into the layer so that ClearType antialiasing can be performed. Rendering ClearType text into a transparent layer does not produce the desired results.'

We then need to know the content type for window surfaces through the API, since we can detect that, and set the correct content type accordingly.

Together with the fix to bug555353 this fixes Direct2D + Aero.
Attachment #447764 - Flags: review?(jmuizelaar)
Assignee

Updated

9 years ago
Blocks: 554874

Comment 1

9 years ago
Will this fix bug 363861?
Comment on attachment 447764 [details] [diff] [review]
Add content type API and initialize cleartype accordingly

Quick questions:

what are the ifdef MOZ_XUL's for?

Does this break cleartype text in the UI? What happens when we only have a single window, instead of separate ones for chrome and content?

It's not clear to me what bug this is fixing. How does the rendering change, before and after and why?
Assignee

Comment 3

9 years ago
(In reply to comment #2)
> (From update of attachment 447764 [details] [diff] [review])
> Quick questions:
> 
> what are the ifdef MOZ_XUL's for?
mTransparencyMode does not exist when !defined(MOZ_XUL)
> 
> Does this break cleartype text in the UI?

It does.

> What happens when we only have a
> single window, instead of separate ones for chrome and content?

I discussed this with roc, we can push an opaque group for content. (Also with layers we'd be using opaque layers, which get cleartype.

> 
> It's not clear to me what bug this is fixing. How does the rendering change,
> before and after and why?

As the clip layer gets pushed, the data from the current layer (say the surface) gets copied to the temp surface for the layer (as the documentation says). Suppose that existing graphical data on the surface has opacity 1.0 or 0.0, this is fine, either it ends up an empty pixel on the new surface, or something solid which will produce correct results when something is drawn to it, and it's then blended back (it will still be solid). However, say there's a pixel with opacity 0.5 (say part of the glow around our text). Gets copied, the layer will end up with opacity 0.5 because of the copying. When the layer is popped it will be composited with an OVER operation. The alpha value in the final surface will now get poluted and take on a larget value as the two partially transparent pixels result in a less transparent pixel as they are blended.
Assignee

Updated

9 years ago
Blocks: 569166

Comment 4

9 years ago
Will this fix the completely transparent alt text box if/when it lands or should that be a separate bug?
Comment on attachment 447764 [details] [diff] [review]
Add content type API and initialize cleartype accordingly


> 
>     props = D2D1::RenderTargetProperties(D2D1_RENDER_TARGET_TYPE_DEFAULT,
>-								       D2D1::PixelFormat(DXGI_FORMAT_UNKNOWN, D2D1_ALPHA_MODE_PREMULTIPLIED),
>-								       dpiX,
>-								       dpiY,
>-								       D2D1_RENDER_TARGET_USAGE_NONE);
>+					 D2D1::PixelFormat(DXGI_FORMAT_UNKNOWN, D2D1_ALPHA_MODE_PREMULTIPLIED),
>+					 dpiX,
>+					 dpiY,
>+					 D2D1_RENDER_TARGET_USAGE_NONE);
>     hr = D2DSurfFactory::Instance()->CreateDxgiSurfaceRenderTarget(newSurf->backBuf,
> 								   props,


Looks like some unrelated indentation changes here?
Attachment #447764 - Flags: review?(jmuizelaar) → review+
Assignee

Comment 6

9 years ago
Pushed http://hg.mozilla.org/mozilla-central/rev/621bb562dbbf.
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.