1285857 - mask-mode: luminance failure on windows with D2D1 backend

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Ethan Lin[:ethlin]]

The is because DataSourceSurfaceD2D1 doesn't support map as MapType::WRITE. 

[1] https://dxr.mozilla.org/mozilla-central/source/layout/base/nsCSSRendering.cpp?q=nsCSSRendering.cpp&redirect_type=direct#5345

Comment 1

[Ethan Lin[:ethlin]]

Attachment: use skia drawtarget

I use the skia drawtarget when the backend is d2d1.

Comment 2

[Markus Stange [:mstange]]

Comment on attachment 8769550 [details] [diff] [review]
use skia drawtarget

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

It would be better to use CreateDrawTargetForData instead of hardcoding backend types like this.

Comment 3

[Ethan Lin[:ethlin]]

Attachment: check if backend support data drawtarget

I couldn't use CreateDrawTargetForData directly since I don't have data at this moment. So I use DoesBackendSupportDataDrawtarget to check if we need to change backend.

Comment 4

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Is there an automated test that fails because of this bug?  If not, it seems that there should be.

Comment 5

[Ethan Lin[:ethlin]]

(In reply to David Baron :dbaron: ⌚️UTC+8 (review requests must explain patch) from comment #4)
> Is there an automated test that fails because of this bug?  If not, it seems
> that there should be.

Yes, the test case should be mask-mode-b.html. But for now we don't have this problem since we have a workaround to switch the backend to SKIA from D2D1 in bug 1275478. After removing the workaround in bug 1275478, we will have this problem. So I think this bug doesn't block mask-image shipping but blocks bug 1275478.

Comment 6

[Markus Stange [:mstange]]

Comment on attachment 8771251 [details] [diff] [review]
check if backend support data drawtarget

Ok, this is fine, thanks.

However, I think we should unify this pattern somewhat. For example, we could have a method called gfxPlatform::CreateSimilarSoftwareDrawTarget, and then the knowledge of the software fallback backend would only exist in gfxPlatform. At the moment, we're littering BackendType::SKIA all over the code and that's not great.

Can you do the cleanup as well, in a separate bug? Thank you.

Comment 7

[Ethan Lin[:ethlin]]

(In reply to Markus Stange [:mstange] from comment #6)
> Comment on attachment 8771251 [details] [diff] [review]
> check if backend support data drawtarget
> 
> Ok, this is fine, thanks.
> 
> However, I think we should unify this pattern somewhat. For example, we
> could have a method called gfxPlatform::CreateSimilarSoftwareDrawTarget, and
> then the knowledge of the software fallback backend would only exist in
> gfxPlatform. At the moment, we're littering BackendType::SKIA all over the
> code and that's not great.
> 
> Can you do the cleanup as well, in a separate bug? Thank you.

Okay, I just opened bug 1289276 to add the function.

Comment 8

[Ethan Lin[:ethlin]]

try server:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=6128ee45d275&selectedJob=24497238

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/0bae57fa83b7
Switch to SKIA from D2D1 when rendering mask image with luminance mode. r=mstange

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/0bae57fa83b7