Only luminance masks need unpremultiplying and conversion to linear RGB

RESOLVED FIXED in mozilla22

Status

()

Core
SVG
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: O S K Chaitanya, Assigned: O S K Chaitanya)

Tracking

({perf})

Trunk
mozilla22
Points:
---
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

4 years ago
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.
(Assignee)

Comment 1

4 years ago
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
Attachment #717549 - Flags: review?(longsonr)
(Assignee)

Comment 2

4 years ago
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

4 years ago
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.

Updated

4 years ago
Summary: nsSVGUtils::UnPremultiplyImageDataAlpha and nsSVGUtils::ConvertImageDataToLinearRGB need only be called when StyleSVGReset()->mMaskType == NS_STYLE_MASK_TYPE_LUMINANCE → Only luminance masks need unpremnsSVGUtils::UnPremultiplyImageDataAlpha and nsSVGUtils::ConvertImageDataToLinearRGB need only be called when StyleSVGReset()->mMaskType == NS_STYLE_MASK_TYPE_LUMINANCE

Updated

4 years ago
Status: UNCONFIRMED → NEW
Ever confirmed: true
Summary: Only luminance masks need unpremnsSVGUtils::UnPremultiplyImageDataAlpha and nsSVGUtils::ConvertImageDataToLinearRGB need only be called when StyleSVGReset()->mMaskType == NS_STYLE_MASK_TYPE_LUMINANCE → Only luminance masks need unpremultiplying and conversion to linear RGB

Updated

4 years ago
Attachment #717549 - Flags: review?(longsonr) → review+

Updated

4 years ago
Flags: in-testsuite-
Keywords: perf
OS: Linux → All
Hardware: x86_64 → All

Comment 4

4 years ago
I can probably land this for you tomorrow.
(Assignee)

Comment 5

4 years ago
Forgive me, but I don't understand what you mean by "land this". Are you offering to commit this patch into trunk?

Comment 6

4 years ago
Yes.

Updated

4 years ago
Assignee: nobody → osk
(Assignee)

Comment 7

4 years ago
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 8

4 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/0ebe5600711d

Comment 9

4 years ago
(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

4 years ago
Oh and thanks for the patch.
https://hg.mozilla.org/mozilla-central/rev/0ebe5600711d
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla22
You need to log in before you can comment on or make changes to this bug.