Closed Bug 445081 Opened 12 years ago Closed 12 years ago

Refactor code to use ComputeNormalizedHypotenuse

Categories

(Core :: SVG, defect)

defect
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: roc, Assigned: roc)

Details

Attachments

(1 file, 1 obsolete file)

Attached patch fix (obsolete) — Splinter Review
A few places compute the expression sqrt((x^2 + y^2)/2). Might as well factor this out even though it's a bit trivial.
Attachment #329383 - Flags: superreview?(mats.palmgren)
Attachment #329383 - Flags: review?(longsonr)
Comment on attachment 329383 [details] [diff] [review]
fix

sr=mats

>+   * Computes sqrt(aWidth^2 + aHeigh^2/2);

I would prefer sqrt((aWidth^2 + aHeight^2)/2) for clarity.

(note the missing 't')
Attachment #329383 - Flags: superreview?(mats.palmgren) → superreview+
Comment on attachment 329383 [details] [diff] [review]
fix

One other opportunity exists in nsSVGGlyphFrame.cpp

    // The context scale is the ratio of the length of the transformed
    // diagonal vector (1,1) to the length of the untransformed diagonal
    // (which is sqrt(2)).
    gfxPoint p = m.Transform(gfxPoint(1, 1)) - m.Transform(gfxPoint(0, 0));
    double contextScale = sqrt((p.x*p.x + p.y*p.y)/2);
Attachment #329383 - Flags: review?(longsonr) → review+
Pushed 4624fef36355 with comments.
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Backed out. I suspect this caused mochitest failures:
*** 12877 ERROR FAIL | text1 char 1 start offset x | got 17, expected 17.000001907348633 | /tests/content/svg/content/test/test_text.html
*** 12879 ERROR FAIL | text1 char 2 start offset x | got 29, expected 29.000003814697266 | /tests/content/svg/content/test/test_text.html
*** 12881 ERROR FAIL | text1 char 0 end offset x | got 17, expected 17.000001907348633 | /tests/content/svg/content/test/test_text.html
*** 12883 ERROR FAIL | text1 char 1 end offset x | got 29, expected 29.000003814697266 | /tests/content/svg/content/test/test_text.html
*** 12885 ERROR FAIL | text1 char 2 end offset x | got 41, expected 41.0000057220459 | /tests/content/svg/content/test/test_text.html
*** 12888 ERROR FAIL | text1 char 0 extent width | got 12, expected 12.000001907348633 | /tests/content/svg/content/test/test_text.html
*** 12891 ERROR FAIL | text1 char 1 extent x | got 17, expected 17.000001907348633 | /tests/content/svg/content/test/test_text.html
*** 12892 ERROR FAIL | text1 char 1 extent width | got 12, expected 12.000001907348633 | /tests/content/svg/content/test/test_text.html
*** 12895 ERROR FAIL | text1 char 2 extent x | got 29, expected 29.000003814697266 | /tests/content/svg/content/test/test_text.html
*** 12896 ERROR FAIL | text1 char 2 extent width | got 12, expected 12.000001907348633 | /tests/content/svg/content/test/test_text.html
*** 12915 ERROR FAIL | text2 char 1 start offset y | got 137, expected 137.00000190734863 | /tests/content/svg/content/test/test_text.html
*** 12917 ERROR FAIL | text2 char 2 start offset y | got 149, expected 149.00000381469727 | /tests/content/svg/content/test/test_text.html
*** 12919 ERROR FAIL | text2 char 0 end offset y | got 137, expected 137.00000190734863 | /tests/content/svg/content/test/test_text.html
*** 12921 ERROR FAIL | text2 char 1 end offset y | got 149, expected 149.00000381469727 | /tests/content/svg/content/test/test_text.html
*** 12923 ERROR FAIL | text2 char 2 end offset y | got 161, expected 161.0000057220459 | /tests/content/svg/content/test/test_text.html
*** 12925 ERROR FAIL | text2 char 0 extent height | got 12, expected 12.000001907348633 | /tests/content/svg/content/test/test_text.html
*** 12928 ERROR FAIL | text2 char 1 extent x | got 137, expected 137.00000190734863 | /tests/content/svg/content/test/test_text.html
*** 12929 ERROR FAIL | text2 char 1 extent width | got 12, expected 12.000001907348633 | /tests/content/svg/content/test/test_text.html
*** 12932 ERROR FAIL | text2 char 2 extent x | got 149, expected 149.00000381469727 | /tests/content/svg/content/test/test_text.html
*** 12933 ERROR FAIL | text2 char 2 extent width | got 12, expected 12.000001907348633 | /tests/content/svg/content/test/test_text.html

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1216092112.1216095075.13838.gz

My theory is that we lost precision in nsSVGGlyphFrame in a way that makes the tests fail.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Attached patch fix v2Splinter Review
Yeah, this fixes the test on my system. I'll reland when I get a chance.
Attachment #329383 - Attachment is obsolete: true
You'll get warnings on Windows about loss of precision converting double to float. Explicit casts would be needed in a couple of places to avoid those.
Relanded with explicit casts added --- changeset de1a57331d93.
Status: REOPENED → RESOLVED
Closed: 12 years ago12 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.