1004375 - [Skia] Remove Moz2D requirement for SK_SUPPORT_LEGACY_COMPATIBLEDEVICE_CONFIG

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[George Wright (:gw280) (needinfo me!)]

Skia are deprecating some of their SkDevice APIs, and to ease the transition the old ones are still available behind an ifdef SK_SUPPORT_LEGACY_COMPATIBLEDEVICE_CONFIG
We should remove our usage of these old APIs and switch to the new ones.

Ideal as an intern bug.

Comment 1

[George Wright (:gw280) (needinfo me!)]

Looks like this is invalid. I don't think we use this API (anymore?)

Comment 2

[Walter Litwinczyk [:walter]]

Attachment: skia_bug_1004375

So it appears that no symbols from the legacy compatible device api, as a full build after this patch produces no problems.

Comment 3

[Walter Litwinczyk [:walter]]

My fault, you have to completely remove the define, as it checks with #ifdef.

Comment 4

[Walter Litwinczyk [:walter]]

Attachment: skia_bug_1004375

Comment 5

[George Wright (:gw280) (needinfo me!)]

Comment on attachment 8427364 [details] [diff] [review]
skia_bug_1004375

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

Looks good. Didn't realise I had already committed a similar patch to moz2d directly (http://hg.mozilla.org/users/bschouten_mozilla.com/moz2d/rev/5e5bb470b61f) but this works just fine :)

Comment 6

[Walter Litwinczyk [:walter]]

Attachment: skia_bug_1004375

Try found a small bug. Create() can return a null value.

Comment 7

[Walter Litwinczyk [:walter]]

New Try: https://tbpl.mozilla.org/?tree=Try&rev=ec20a8970919

I think Linux Debug bc1 will eventually pass, just unlucky problems.

Comment 8

[Walter Litwinczyk [:walter]]

It appears there may be a real bug on B2G Desktop OS X Opt Gu, will take a look into that.

Comment 9

[Walter Litwinczyk [:walter]]

Okay, so the failure does not appear to be from this patch: https://bugzilla.mozilla.org/show_bug.cgi?id=1027604

Comment 10

[George Wright (:gw280) (needinfo me!)]

Comment on attachment 8445519 [details] [diff] [review]
skia_bug_1004375

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

::: gfx/2d/DrawTargetSkia.cpp
@@ +708,5 @@
>  bool
>  DrawTargetSkia::Init(const IntSize &aSize, SurfaceFormat aFormat)
>  {
> +  SkAlphaType alphaType = (aFormat == SurfaceFormat::B8G8R8X8) ?
> +    kOpaque_SkAlphaType : kPremul_SkAlphaType;

I think it should be kIgnore_SkAlphaType

Comment 11

[Walter Litwinczyk [:walter]]

I did a test with kIgnore_SkAlphaType: https://tbpl.mozilla.org/?tree=Try&rev=66b15857281d (ignore the whitespace changes) and it seemed to create lots of pixel differences. Since the first one works better in try (at the moment) I'll mark it as checkin-needed.

Comment 12

[Carsten Book [:Tomcat]]

Hi Walter,

this patch fails to apply during checkin:

applying skia_bug_1004375
patching file gfx/skia/trunk/include/config/SkUserConfig.h
Hunk #1 FAILED at 200
1 out of 1 hunks FAILED -- saving rejects to file gfx/skia/trunk/include/config/SkUserConfig.h.rej
patch failed, unable to continue (try -v)
patch failed, rejects left in working dir
errors during apply, please fix and refresh skia_bug_1004375

could you take a look, thanks!

Comment 13

[Walter Litwinczyk [:walter]]

Attachment: Should fix the failed to apply

r=gw280 carried forward. Fixed patch apply issues.

Hi Carsten,

It appears there was a small dependency between this patch and bug 1004374. There was a line removed from the header in that patch that was in the context of the diff for this patch. It should apply cleanly now.

Comment 14

[Walter Litwinczyk [:walter]]

Attachment: Should apply cleanly now

r=gw280 carried forward.

Not a good morning, somehow uploaded the wrong patch... Above comments still apply.

Comment 15

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/0f7f4d5f5e45

Comment 16

[Carsten Book [:Tomcat]]

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