Closed Bug 910754 Opened 7 years ago Closed 6 years ago

[Skia] Update Skia, 2014Q1

Categories

(Core :: Graphics, enhancement)

x86_64
Linux
enhancement
Not set

Tracking

()

RESOLVED FIXED
mozilla30

People

(Reporter: gw280, Assigned: gw280)

References

(Depends on 1 open bug, Blocks 1 open bug)

Details

Attachments

(16 files)

6.81 KB, patch
mattwoodrow
: review+
Details | Diff | Splinter Review
11.31 KB, patch
vlad
: review+
Details | Diff | Splinter Review
1.19 KB, patch
snorp
: review+
Details | Diff | Splinter Review
5.11 KB, patch
snorp
: review+
Details | Diff | Splinter Review
1.25 KB, patch
snorp
: review+
Details | Diff | Splinter Review
1.46 KB, patch
snorp
: review+
Details | Diff | Splinter Review
845 bytes, patch
snorp
: review+
Details | Diff | Splinter Review
2.79 KB, patch
snorp
: review+
Details | Diff | Splinter Review
1.19 KB, patch
snorp
: review+
Details | Diff | Splinter Review
830 bytes, patch
snorp
: review+
Details | Diff | Splinter Review
1.02 KB, patch
snorp
: review+
Details | Diff | Splinter Review
41.28 KB, patch
snorp
: review+
Details | Diff | Splinter Review
983 bytes, patch
snorp
: review+
Details | Diff | Splinter Review
531 bytes, patch
vlad
: review+
Details | Diff | Splinter Review
1.07 KB, patch
vlad
: review+
Details | Diff | Splinter Review
1.66 KB, patch
snorp
: review+
Details | Diff | Splinter Review
Let's do another Skia update. Most important new features we'd like to pull in are fixes to Skia/GL's texture cache to stop it from overrunning its cache limits, and advanced xfermodes which we'd like to use.
If this is to end up in 26, convince me we have time to do it :)
Attached patch shadows.patchSplinter Review
Forward port Matt's patch from bug 884888
Attachment #810838 - Flags: review?(matt.woodrow)
Comment on attachment 810838 [details] [diff] [review]
shadows.patch

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

Do you need the changes to drawRect to ensure that we actually end up inside drawPath?
Attachment #810838 - Flags: review?(matt.woodrow) → review+
Good catch. I thought I'd applied that hunk, but I guess I didn't.
Summary: [Skia] Update Skia, 2013Q3 → [Skia] Update Skia, 2014Q1
Comment on attachment 8377702 [details] [diff] [review]
0001-Bug-910754-Add-a-bunch-more-required-OpenGL-function.patch

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

Looks good, but can you add a comment above the block of functions in the two headers saying that this is functionality needed for Skia, and should not be generally used?  This stuff is not in ES2.0, it's only desktop Skia that's going to get or use this.  I almost wish we could create a context and pass a flag to say "this is a context for skia" that would cause us to look up this stuff.
Attachment #8377702 - Flags: review?(vladimir) → review+
Comment on attachment 8377703 [details] [diff] [review]
0002-Bug-910754-skia-npapi-s-anp_getFontPath-function-doe.patch

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

Are you sure we don't need this anymore? IIRC, Flash didn't work anymore without this? Did you try on 2.3 and 4.0+?
Attachment #8377703 - Flags: review?(snorp)
Attachment #8377705 - Flags: review?(snorp) → review+
Attachment #8377706 - Flags: review?(snorp) → review+
Comment on attachment 8377707 [details] [diff] [review]
0006-Bug-910754-Mark-linear-gradient-1a-and-linear-gradie.patch

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

Alright I guess, but what platform was this new failing on?
Attachment #8377707 - Flags: review?(snorp) → review+
Attachment #8377708 - Flags: review?(snorp) → review+
Attachment #8377709 - Flags: review?(snorp) → review+
Attachment #8377711 - Flags: review?(snorp) → review+
(In reply to James Willcox (:snorp) (jwillcox@mozilla.com) from comment #19)
> Alright I guess, but what platform was this new failing on?

This was discussed between myself and jrmuizel; we decided that as other backends like Quartz etc were already failing these tests, and other browsers fail it anyway (Chrome and Safari do), we shouldn't care about it anymore.
Attachment #8377712 - Flags: review?(snorp) → review+
Attachment #8377713 - Flags: review?(snorp) → review+
(In reply to James Willcox (:snorp) (jwillcox@mozilla.com) from comment #18)
> Are you sure we don't need this anymore? IIRC, Flash didn't work anymore
> without this? Did you try on 2.3 and 4.0+?

Flash works fine on 4.x with this. I can't comment on 2.x.
Attachment #8377714 - Flags: review?(snorp) → review+
Attachment #8377883 - Flags: review?(snorp) → review+
Depends on: 974272
Depends on: 974360
Depends on: 974335
Attached patch mozbuild-fixupSplinter Review
derp, I forgot to add that last moz.build fixup to the generation script :/
Attachment #8378565 - Flags: review?(snorp)
Comment on attachment 8378565 [details] [diff] [review]
mozbuild-fixup

land away, include NPOTB to not trigger a build though. (NPOB? NPOT? I can never remember!)
Attachment #8378565 - Flags: review?(snorp) → review+
DONTBUILD I think?
Attached patch headerSplinter Review
Add a warning header to moz.build to signify that the file shouldn't be modified directly.
Attachment #8378577 - Flags: review?(vladimir)
This un-unified the Skia build: the UNIFIED_SOURCES in gfx/skia/moz.build were overwritten with just SOURCES. This is a big deal because skia build unification (bug 939629) shaved 1 minute cpu time off the build.
Are we using this latest skia version in Master branch now (for firefox OS platform)? Can you please confirm us ?
Flags: needinfo?(gwright)
Please also confirm future skia version update plans for v1.4 and Master (for firefox os platform) .
Flags: needinfo?(snorp)
(In reply to Tapas Kumar Kundu from comment #33)
> Please also confirm future skia version update plans for v1.4 and Master
> (for firefox os platform) .

The version of Skia in master (and 1.4) is r13424.
Flags: needinfo?(snorp)
Future skia update plans are to pull in a new version once per train. i.e. every 6 weeks or so. I also plan to do a quick interim testing update and push to my personal branches + graphics branch once per week (tentatively on Fridays).

As snorp said, current master branch is using Skia from two weeks ago (r13424).
Flags: needinfo?(gwright)
Comment on attachment 8390473 [details] [diff] [review]
0001-Bug-910754-Use-SkMemory_mozalloc-instead-r-snorp.patch

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

I guess if we have this file in generate_mozbuild.py, future rebases will fail without it, so we don't need to upstream that.
Attachment #8390473 - Flags: review?(snorp) → review+
(In reply to James Willcox (:snorp) (jwillcox@mozilla.com) from comment #38)
> I guess if we have this file in generate_mozbuild.py, future rebases will
> fail without it, so we don't need to upstream that.

SkMemory_mozalloc was upstreamed months ago :)
Depends on: 1023732
Depends on: 1028827
You need to log in before you can comment on or make changes to this bug.