Last Comment Bug 844511 - Only luminance masks need unpremultiplying and conversion to linear RGB
: Only luminance masks need unpremultiplying and conversion to linear RGB
Status: RESOLVED FIXED
: perf
Product: Core
Classification: Components
Component: SVG (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla22
Assigned To: O S K Chaitanya
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2013-02-23 13:49 PST by O S K Chaitanya
Modified: 2013-02-24 19:58 PST (History)
1 user (show)
longsonr: in‑testsuite-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
svg_UnPreMultiplyImageDataAlpha_only_for_relevant_masktype.patch (2.95 KB, patch)
2013-02-23 13:49 PST, O S K Chaitanya
longsonr: review+
Details | Diff | Splinter Review

Description O S K Chaitanya 2013-02-23 13:49:32 PST
Created attachment 717549 [details] [diff] [review]
svg_UnPreMultiplyImageDataAlpha_only_for_relevant_masktype.patch

User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:19.0) Gecko/20100101 Firefox/19.0
Build ID: 20130218103317

Steps to reproduce:

Studied nsSVGMaskFrame.cpp.


Actual results:

nsSVGUtils::UnPremultiplyImageDataAlpha and nsSVGUtils::ConvertImageDataToLinearRGB are called before checking what  StyleSVGReset()->mMaskType == NS_STYLE_MASK_TYPE_LUMINANCE returns.


Expected results:

They should only be called when it returns true.
Comment 1 O S K Chaitanya 2013-02-23 13:51:25 PST
Comment on attachment 717549 [details] [diff] [review]
svg_UnPreMultiplyImageDataAlpha_only_for_relevant_masktype.patch

Moved the calls to nsSVGUtils::UnPremultiplyImageDataAlpha and nsSVGUtils::ConvertImageDataToLinearRGB to the block corresponding to StyleSVG()->mColorInterpolation == NS_STYLE_COLOR_INTERPOLATION_LINEARRGB being true
Comment 2 O S K Chaitanya 2013-02-23 13:53:09 PST
It would be very nice if someone could point me to some any existing test cases for the masking related code in SVG.
Comment 3 Robert Longson 2013-02-23 14:03:03 PST
layout/reftests/svg/mask-type-01.svg layout/reftests/svg/mask-type-02.svg layout/reftests/svg/mask-type-03.svg all test alpha masks. The other files in layout/reftests/svg with mask in their titles test luminance masks.
Comment 4 Robert Longson 2013-02-23 14:32:26 PST
I can probably land this for you tomorrow.
Comment 5 O S K Chaitanya 2013-02-23 15:08:58 PST
Forgive me, but I don't understand what you mean by "land this". Are you offering to commit this patch into trunk?
Comment 6 Robert Longson 2013-02-23 15:20:57 PST
Yes.
Comment 7 O S K Chaitanya 2013-02-23 15:36:46 PST
Thank you! So, is it now safe to assume this means I need not add the "checkin-needed" keyword mentioned on this page: https://developer.mozilla.org/en-US/docs/Developer_Guide/How_to_Submit_a_Patch ?
Comment 9 Robert Longson 2013-02-24 01:09:31 PST
(In reply to O S K Chaitanya from comment #7)
> Thank you! So, is it now safe to assume this means I need not add the
> "checkin-needed" keyword mentioned on this page:
> https://developer.mozilla.org/en-US/docs/Developer_Guide/
> How_to_Submit_a_Patch ?

If you want to add checkin-needed your patch ought to follow these rules: https://developer.mozilla.org/en-US/docs/Creating_a_patch_that_can_be_checked_in

As it's your first time I let you off jumping through those hoops ;-)

Your patch is on the mozilla-inbound trunk tree now and should make it to mozilla-central within the next 24 hours.
Comment 10 Robert Longson 2013-02-24 01:16:17 PST
Oh and thanks for the patch.
Comment 11 Phil Ringnalda (:philor) 2013-02-24 19:58:06 PST
https://hg.mozilla.org/mozilla-central/rev/0ebe5600711d

Note You need to log in before you can comment on or make changes to this bug.