Make the SMIL animate motion code use a Moz2D PathBuilder instead of gfxContext

RESOLVED FIXED in mozilla28

Status

()

Core
SVG
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: bas, Assigned: jwatt)

Tracking

(Depends on: 1 bug, Blocks: 2 bugs)

unspecified
mozilla28
x86_64
Windows 8
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [qa-])

Attachments

(2 attachments, 1 obsolete attachment)

Comment hidden (empty)
(Reporter)

Comment 1

4 years ago
Created attachment 831289 [details] [diff] [review]
Part 2: Mark some tests fuzzy, and change the discontinuity side of joins
Attachment #831289 - Flags: review?(dholbert)
(Reporter)

Comment 2

4 years ago
Created attachment 831290 [details] [diff] [review]
Part 1: Change SMIL animations to use Moz2D Paths

This patch is Matt Woodrow's, I'd review it myself but figured Jwatt might like a quick look at it. I realize the last hunk can be done a lot better but we can do that in a follow-up if we want.
Attachment #831290 - Flags: review?(jwatt)
Comment on attachment 831289 [details] [diff] [review]
Part 2: Mark some tests fuzzy, and change the discontinuity side of joins

>+++ b/content/svg/content/test/test_text.html
>@@ -99,25 +99,25 @@ function runTest()
>-  is(text2.getExtentOfChar(0).height, charWidth, "text2 char 0 extent height");
>+  isfuzzy(text2.getExtentOfChar(0).height, charWidth, 0.000001, "text2 char 0 extent height");
[etc]

Where is "isfuzzy" defined? I don't see it listed alongside is, todo, etc. here:
 http://mxr.mozilla.org/mozilla-central/source/testing/mochitest/tests/SimpleTest/SimpleTest.js#1136
and an MXR search finds nothing:
 http://mxr.mozilla.org/mozilla-central/search?string=isfuzzy&case=on&tree=mozilla-central

Perhaps you have a patch elsewhere that adds it?
(Reporter)

Comment 4

4 years ago
(In reply to Daniel Holbert [:dholbert] from comment #3)
> Comment on attachment 831289 [details] [diff] [review]
> Part 2: Mark some tests fuzzy, and change the discontinuity side of joins
> 
> >+++ b/content/svg/content/test/test_text.html
> >@@ -99,25 +99,25 @@ function runTest()
> >-  is(text2.getExtentOfChar(0).height, charWidth, "text2 char 0 extent height");
> >+  isfuzzy(text2.getExtentOfChar(0).height, charWidth, 0.000001, "text2 char 0 extent height");
> [etc]
> 
> Where is "isfuzzy" defined? I don't see it listed alongside is, todo, etc.
> here:
>  http://mxr.mozilla.org/mozilla-central/source/testing/mochitest/tests/
> SimpleTest/SimpleTest.js#1136
> and an MXR search finds nothing:
>  http://mxr.mozilla.org/mozilla-central/
> search?string=isfuzzy&case=on&tree=mozilla-central
> 
> Perhaps you have a patch elsewhere that adds it?

Bug 937678
Attachment #831289 - Flags: review?(dholbert) → review+
Comment on attachment 831290 [details] [diff] [review]
Part 1: Change SMIL animations to use Moz2D Paths

This is all covered by the much more extensive patches in bug 934305 and bug 938388.
Attachment #831290 - Flags: review?(jwatt) → review-
Created attachment 831880 [details] [diff] [review]
patch - Make the SMIL animate motion code use a Moz2D PathBuilder instead of gfxContext

Oh, wait. I'm juggling so many convert-SVG-to-Moz2D patches that I knocked out during the rendering work week that I'm getting confused. Some of this stuff is in an intermediate patch that I failed to make a bug for. This bug actually matches what that patch does. I'm going to ask for review on my patch though since I know it doesn't conflict with the others I wrote (and Matt should have been ready for that to happen since he knew I was knocking out SVG-to-Moz2D patches all over the SVG code before he wrote his patch ;) ).
Assignee: matt.woodrow → jwatt
Attachment #831290 - Attachment is obsolete: true
Attachment #831880 - Flags: review?(dholbert)
(Assignee)

Updated

4 years ago
Blocks: 703159, 926258
Depends on: 935049
Summary: Change SMIL animation code to use Moz2D paths → Make the SMIL animate motion code use a Moz2D PathBuilder instead of gfxContext
I'm totally fine with that, I just wrote this so we could get coverage of the new path measurement code.

Bas is really keen to get his patches landed, and exercised during tests, so it would be appreciated if you could get your equivalents landed asap.
Ah, totally makes sense. I should have done a better job of marking dependencies in which case you'd have found it easier to find patches that depended on Bas' work so you could test it. I was just thinking it best not to get reviews on most of them until Bas' work had landed and I could test it though. :)

For future reference, do note that I have a bunch of SVG patches in the wings, so if you need something to test new Moz2D features ping me and if I have anything I'll tidy it up for you.
Comment on attachment 831880 [details] [diff] [review]
patch - Make the SMIL animate motion code use a Moz2D PathBuilder instead of gfxContext

># HG changeset patch
># Parent 6d57a24482a57477e42757bd1796479f235760eb
># User Jonathan Watt <jwatt@jwatt.org>
>Bug 937994 - Make the SMIL animate motion code use a Moz2D PathBuilder instead of gfxContext. r=dholbert

s/animate motion/animateMotion/

r=me with that
Attachment #831880 - Flags: review?(dholbert) → review+
(Assignee)

Updated

4 years ago
Depends on: 937678
https://hg.mozilla.org/integration/mozilla-inbound/rev/df19d2da7e2e
https://hg.mozilla.org/integration/mozilla-inbound/rev/a08baab2e6f3
https://hg.mozilla.org/mozilla-central/rev/df19d2da7e2e
https://hg.mozilla.org/mozilla-central/rev/a08baab2e6f3
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla28

Updated

4 years ago
Whiteboard: [qa-]
You need to log in before you can comment on or make changes to this bug.