Summary: | Apply feTurbulence spec change to fix zero length vector generation | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Nikos Andronikos <nikos.andronikos> | ||||||
Component: | SVG | Assignee: | Nikos Andronikos <nikos.andronikos> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | commit-queue, dino, kondapallykalyan, krit, zimmermann | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Attachments: |
|
Description
Nikos Andronikos
2015-01-22 19:31:21 PST
Created attachment 245202 [details]
Patch
Comment on attachment 245202 [details]
Patch
Looks good to me. I am not happy to submit it without a test. IIRC there were some test cases on public-fx or www-svg. Could you add one of these please? r- because of missing test.
Created attachment 245393 [details]
Patch
(In reply to comment #2) > Comment on attachment 245202 [details] > Patch > > Looks good to me. I am not happy to submit it without a test. IIRC there > were some test cases on public-fx or www-svg. Could you add one of these > please? r- because of missing test. Ok, done. I was a bit worried about adding a pixel test but managed to come up with a way to use a reftest for this. Comment on attachment 245393 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=245393&action=review > Source/WebCore/platform/graphics/filters/FETurbulence.cpp:186 > + do { > + gradient[0] = static_cast<float>((paintingData.random() % (2 * s_blockSize)) - s_blockSize) / s_blockSize; > + gradient[1] = static_cast<float>((paintingData.random() % (2 * s_blockSize)) - s_blockSize) / s_blockSize; > + } while (!gradient[0] && !gradient[1]); Is a loop really the best way to avoid zeroes here? Comment on attachment 245393 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=245393&action=review >> Source/WebCore/platform/graphics/filters/FETurbulence.cpp:186 >> + } while (!gradient[0] && !gradient[1]); > > Is a loop really the best way to avoid zeroes here? I guess that’s what the specification calls for. (In reply to comment #6) > Comment on attachment 245393 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=245393&action=review > > >> Source/WebCore/platform/graphics/filters/FETurbulence.cpp:186 > >> + } while (!gradient[0] && !gradient[1]); > > > > Is a loop really the best way to avoid zeroes here? > > I guess that’s what the specification calls for. It does =) The idea is that the random distribution is maintained. Falling back to some known value would skew the result (but only very slightly in this case). Discussion was here: https://lists.w3.org/Archives/Public/www-svg/2015Jan/0005.html Thanks for the review. Comment on attachment 245393 [details] Patch Clearing flags on attachment: 245393 Committed r179171: <http://trac.webkit.org/changeset/179171> All reviewed patches have been landed. Closing bug. |