Closed
Bug 1359527
Opened 8 years ago
Closed 7 years ago
NYTimes SVG can take 30ms to render
Categories
(Core :: Graphics, defect)
Core
Graphics
Tracking
()
RESOLVED
FIXED
mozilla55
Tracking | Status | |
---|---|---|
firefox55 | --- | fixed |
People
(Reporter: mchang, Assigned: mchang)
References
(Depends on 2 open bugs)
Details
Attachments
(5 files, 9 obsolete files)
(deleted),
image/svg+xml
|
Details | |
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
mchang
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
jrmuizel
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
mchang
:
review+
|
Details | Diff | Splinter Review |
Profile - https://perfht.ml/2qbuQqZ
Assignee | ||
Comment 1•8 years ago
|
||
Still a WIP dump before I forget this week on PTO. I tried making the callers of GetMaskForMaskedFrame assume what was returned was an RGBA surface, then use push/pop layer to convert it to an alpha mask using luminance. However, I had lots of flags being pushed around like whether to convert it to an alpha mask, which SVG type or create a new flag about how to create an alpha mask. It wasn't I' very pretty. Also, sometimes the mask wasn't directy used in push/pop groups, so I had to feed this information down to nsSVGPath as it sometimes had an extra mask. The code got quite ugly.
I wanted to try to do something like DrawTarget::SnapshotAlpha, that would give you an alpha surface based on the SVG type. This worked great for everything except D2D. I've been fighting D2D to draw the luminance effect to a temporary surface, since D2D can't draw an effect from the texture it's reading from. However, this doesn't seem to be working which is where I'm at now.
Assignee | ||
Comment 2•8 years ago
|
||
Comment on attachment 8863172 [details] [diff] [review]
WIP - Create an alpha snapshot option for SVG Masks
Just wondering if you could take a look at drawTargetD2D1::SnapshotAlpha here. What I'd like to do is create a temporary bitmap with the current contents after a luminance d2d effect. This doesn't seem to be working and the resulting surface always seems zeroed out. Does this not work in general or am I missing something here? Thanks!
Attachment #8863172 -
Flags: feedback?(bas)
Assignee | ||
Updated•8 years ago
|
Attachment #8863172 -
Flags: feedback?(bas)
Assignee | ||
Comment 3•8 years ago
|
||
Assignee | ||
Comment 4•8 years ago
|
||
Any idea why DrawTargetD2D1::SnapshotAlpha doesn't seem to do anything?
Attachment #8863172 -
Attachment is obsolete: true
Attachment #8866987 -
Flags: feedback?(jmuizelaar)
Comment 5•8 years ago
|
||
Comment on attachment 8866987 [details] [diff] [review]
DrawTarget::SnapshotAlpha
It seems reasonable to me but Bas probably has better intuition.
Attachment #8866987 -
Flags: feedback?(jmuizelaar) → feedback?(bas)
Comment 6•8 years ago
|
||
Comment on attachment 8866987 [details] [diff] [review]
DrawTarget::SnapshotAlpha
Review of attachment 8866987 [details] [diff] [review]:
-----------------------------------------------------------------
::: gfx/2d/DrawTargetD2D1.cpp
@@ +123,5 @@
> + }
> +
> + // Create the luminance effect
> + RefPtr<ID2D1Effect> luminanceEffect;
> + hr = mDC->CreateEffect(CLSID_D2D1LuminanceToAlpha, getter_AddRefs(luminanceEffect));
Note you'll want to cache this effect, effect creation is relatively expensive.
@@ +141,5 @@
> + // Draw the output of the luminance to the tmp bitmap
> + Rect fullRect(0, 0, mSize.width, mSize.height);
> + mDC->SetTransform(D2D1::IdentityMatrix());
> + mDC->SetTarget(tmpBitmap);
> + mDC->DrawImage(luminanceImage);
This is probably your problem, I'm guessing GetOutput isn't implemented well for this effect. You can implement the same thing more efficiently by using:
https://msdn.microsoft.com/en-us/library/windows/desktop/jj841149(v=vs.85).aspx
Instead of getting the output image and then drawing that. Let me know how that goes, otherwise I'll have some other ideas.
Updated•8 years ago
|
Attachment #8866987 -
Flags: feedback?(bas)
Assignee | ||
Comment 7•8 years ago
|
||
Drawing the luminance effect directly doesn't seem to work exactly as needed as well :/.
Attachment #8868265 -
Flags: feedback?(bas)
Comment 8•8 years ago
|
||
(In reply to Mason Chang [:mchang] from comment #7)
> Created attachment 8868265 [details] [diff] [review]
> Directly draw the luminance effect
>
> Drawing the luminance effect directly doesn't seem to work exactly as needed
> as well :/.
Define 'doesn't seem to work exactly as needed'? :) That's relatively little information to go on.
Comment 9•8 years ago
|
||
Comment on attachment 8868265 [details] [diff] [review]
Directly draw the luminance effect
Review of attachment 8868265 [details] [diff] [review]:
-----------------------------------------------------------------
::: gfx/2d/DrawTargetD2D1.cpp
@@ +146,5 @@
> + mDC->DrawImage(luminanceEffect,
> + nullptr,
> + nullptr,
> + D2D1_INTERPOLATION_MODE_LINEAR,
> + D2D1_COMPOSITE_MODE_SOURCE_OVER);
At least you're not clearing the bitmap so you'll want to use COPY, but I doubt that's your issue.
Attachment #8868265 -
Flags: feedback?(bas)
Assignee | ||
Updated•8 years ago
|
Assignee | ||
Comment 10•8 years ago
|
||
Seems to work other than one stylo maybe reftest failure - https://treeherder.mozilla.org/#/jobs?repo=try&revision=94e7847589c32ce6572ef284ee90b7d7d9427aeb
Attachment #8866987 -
Attachment is obsolete: true
Attachment #8868265 -
Attachment is obsolete: true
Assignee | ||
Comment 11•8 years ago
|
||
Comment on attachment 8869468 [details] [diff] [review]
Working SnapshotLuminance patch
Looks like the failure was an intermittent.
Attachment #8869468 -
Flags: review?(jmuizelaar)
Comment 12•8 years ago
|
||
Comment on attachment 8869468 [details] [diff] [review]
Working SnapshotLuminance patch
Review of attachment 8869468 [details] [diff] [review]:
-----------------------------------------------------------------
Bas should review this.
Attachment #8869468 -
Flags: review?(jmuizelaar) → review?(bas)
Comment 13•8 years ago
|
||
Comment on attachment 8869468 [details] [diff] [review]
Working SnapshotLuminance patch
Review of attachment 8869468 [details] [diff] [review]:
-----------------------------------------------------------------
::: gfx/2d/2D.h
@@ +896,5 @@
> virtual already_AddRefed<SourceSurface> Snapshot() = 0;
> +
> + // Snapshots the contents and returns an alpha mask
> + // based on the RGB values.
> + virtual already_AddRefed<SourceSurface> SnapshotLuminance(uint8_t aMaskParams, float aOpacity);
Since this is mutating the DrawTarget calling it Snapshot is probably not the best. Maybe IntoLuminanceSource()?
::: gfx/2d/DrawTarget.cpp
@@ +288,5 @@
> + default:
> + {
> + ComputeAlphaMask(map.mData, map.mStride,
> + destMap.mData, destMap.mStride,
> + size, aOpacity);
Shouldn't the users of this just be drawing to an A8 surface?
Assignee | ||
Comment 14•8 years ago
|
||
Comment on attachment 8869468 [details] [diff] [review]
Working SnapshotLuminance patch
Review of attachment 8869468 [details] [diff] [review]:
-----------------------------------------------------------------
::: gfx/2d/DrawTarget.cpp
@@ +288,5 @@
> + default:
> + {
> + ComputeAlphaMask(map.mData, map.mStride,
> + destMap.mData, destMap.mStride,
> + size, aOpacity);
What do you mean? We create an A8 surface above and draw to it.
Comment 15•8 years ago
|
||
Comment on attachment 8869468 [details] [diff] [review]
Working SnapshotLuminance patch
Review of attachment 8869468 [details] [diff] [review]:
-----------------------------------------------------------------
::: gfx/2d/DrawTarget.cpp
@@ +288,5 @@
> + default:
> + {
> + ComputeAlphaMask(map.mData, map.mStride,
> + destMap.mData, destMap.mStride,
> + size, aOpacity);
If the producer of the mask knows that they are going to need an alpha mask they should just create an A8 DrawTarget. Also the code in ComputeAlphaMask is terribly slow. Even though it's just being moved I object to existence in the first place :) I'll see if I can write a patch that removes it.
Comment 16•8 years ago
|
||
Comment on attachment 8869468 [details] [diff] [review]
Working SnapshotLuminance patch
Review of attachment 8869468 [details] [diff] [review]:
-----------------------------------------------------------------
Also, have you measured a performance improvement with this patch?
Assignee | ||
Comment 17•8 years ago
|
||
(In reply to Jeff Muizelaar [:jrmuizel] from comment #16)
> Comment on attachment 8869468 [details] [diff] [review]
> Working SnapshotLuminance patch
>
> Review of attachment 8869468 [details] [diff] [review]:
> -----------------------------------------------------------------
>
> Also, have you measured a performance improvement with this patch?
I did and it is all over the place. Sometimes it's much faster, sometimes its slower. I can't reproduce the slowness on NYT on my Windows machine since probably the page changed. My giant Snapchat mask has huge variability, sometimes 2ms to 60 ms. The old CPU version goes from 30-40ms, so this might be a case as with box shadows where certain sizes aren't worth making a temporary bitmap.
Assignee | ||
Comment 18•8 years ago
|
||
I tried to wrap the output of the Luminance Image with a SourceSurfaceD2D1. But according to the docs [1], the output sets the RGB channels to 0 and only sets the alpha channel. If we directly use the output of the luminance effect, it sets the bitmap's RGB channels to 0.
This worked with push/pop group because we should first Snapshot the full bitmap, which creates a copy in SourceSurface[2]. This copy would then become the input to the luminance effect, and the output of the luminance effect with the copy would become the mask. It was never directly touching the mBitmap of DrawTargetD2D.
[1] https://msdn.microsoft.com/en-us/library/windows/desktop/hh706363(v=vs.85).aspx
[2] http://searchfox.org/mozilla-central/source/gfx/2d/SourceSurfaceD2D1.cpp#97
Comment 19•8 years ago
|
||
As I understand it, effects don't mutate their inputs. Also, I think the copy in SourceSurface only happens if original DrawTarget changes.
Comment 20•8 years ago
|
||
Assignee | ||
Comment 21•8 years ago
|
||
(In reply to Jeff Muizelaar [:jrmuizel] from comment #19)
> As I understand it, effects don't mutate their inputs. Also, I think the
> copy in SourceSurface only happens if original DrawTarget changes.
I found the bug here if we do not create a temporary surface and instead only wrap the output of the luminance effect in SnapshotLuminance. When we call DrawTargetD2D1::PushLayer, we create a bitmap brush and use that as the mask [1]. However, the query interface actually fails, so the bitmap brush becomes empty. It looks like if you're given an actual ID2D1Image, you aren't allowed to query interface [2] for an ID2D1Bitmap. From the test cases I've seen, usually when we call PushLayer, we're given a DataSourceSurface, which creates an ID2D1Bitmap and thus we can queryInterface for it successfully. I think all calls at [3] where the result is an actual ID2D1Effect will actually fail here. From the MSDN docs [4], we can only use an ID2D1ImageBrush on Windows 8 and later, which means we'd have to use a temporary surface on Windows 7 to draw the image to a bitmap, then create the bitmap brush.
A workaround could be to check what the actual mask surface is, and create a bitmap brush if we have a data surface. If we have an ID2D1Image, we can create an image brush on windows 8 and higher and create a temporary surface on windows 7 and lower.
I wanted to see if this failed anywhere else in master before though - try with a MOZ_CRASH if query_interface fails - https://treeherder.mozilla.org/#/jobs?repo=try&revision=904483673de29e61bff21d9171bae8c96be5cb6a
[1] http://searchfox.org/mozilla-central/source/gfx/2d/DrawTargetD2D1.cpp#841
[2] https://stackoverflow.com/questions/31118397/direct2d-convert-id2d1image-to-id2d1bitmap/31165846
[3] http://searchfox.org/mozilla-central/source/gfx/2d/DrawTargetD2D1.cpp#832
[4] https://msdn.microsoft.com/en-us/library/windows/desktop/dd756651(v=vs.85).aspx
Comment 22•8 years ago
|
||
I think we should be able to just switch to using an image brush unconditionally. I believe the Windows 8 functionality is also available in Win7 with platform update which is required for D2D these days.
Assignee | ||
Comment 23•8 years ago
|
||
Attachment #8866549 -
Attachment is obsolete: true
Attachment #8869468 -
Attachment is obsolete: true
Attachment #8869468 -
Flags: review?(bas)
Attachment #8870511 -
Flags: review?(jmuizelaar)
Attachment #8870511 -
Flags: review?(bas)
Comment 24•8 years ago
|
||
Comment on attachment 8870511 [details] [diff] [review]
Add DrawTarget::IntoLuminance
Review of attachment 8870511 [details] [diff] [review]:
-----------------------------------------------------------------
::: gfx/2d/2D.h
@@ +896,5 @@
> virtual already_AddRefed<SourceSurface> Snapshot() = 0;
> +
> + // Snapshots the contents and returns an alpha mask
> + // based on the RGB values.
> + virtual already_AddRefed<SourceSurface> IntoLuminanceSource(uint8_t aMaskParams, float aOpacity);
MaskParams should be an enum
::: gfx/2d/DrawTargetD2D1.cpp
@@ +17,5 @@
> #include "Tools.h"
>
> +#include "nsStyleConsts.h"
> +#include "nsAppRunner.h"
> +#include "nsDebug.h"
We can't include these headers in Moz2D
@@ +125,5 @@
> + }
> +
> + // Make sure we flush everything.
> + // This will call BeginDraw for us
> + Flush();
Is this needed?
::: gfx/2d/SourceSurfaceD2D1.cpp
@@ +6,5 @@
> #include "SourceSurfaceD2D1.h"
> #include "DrawTargetD2D1.h"
> #include "Tools.h"
> +#include "nsDebug.h"
> +#include "nsAppRunner.h"
We can't include these headers in Moz2D
Attachment #8870511 -
Flags: review?(jmuizelaar) → review-
Assignee | ||
Comment 25•8 years ago
|
||
Updated with fixes from comment 24.
Attachment #8870511 -
Attachment is obsolete: true
Attachment #8870511 -
Flags: review?(bas)
Attachment #8870541 -
Flags: review?(jmuizelaar)
Attachment #8870541 -
Flags: review?(bas)
Assignee | ||
Comment 26•8 years ago
|
||
Attachment #8870541 -
Attachment is obsolete: true
Attachment #8870541 -
Flags: review?(jmuizelaar)
Attachment #8870541 -
Flags: review?(bas)
Attachment #8870542 -
Flags: review?(jmuizelaar)
Attachment #8870542 -
Flags: review?(bas)
Updated•8 years ago
|
Attachment #8870542 -
Flags: review?(jmuizelaar) → review+
Comment 27•8 years ago
|
||
Comment on attachment 8870542 [details] [diff] [review]
Add DrawTarget::IntoLuminance
Review of attachment 8870542 [details] [diff] [review]:
-----------------------------------------------------------------
Bas reviewed this over my shoulder. He said it looked fine.
Attachment #8870542 -
Flags: review?(bas) → review+
Comment 28•8 years ago
|
||
I landed bug 1367147 which conflicts with this. Here's a rebased version.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=9281b1e567329b55f3a677780e7d1d9ad4c94098
Attachment #8870542 -
Attachment is obsolete: true
Attachment #8872009 -
Flags: review+
Comment 29•8 years ago
|
||
Comment 30•8 years ago
|
||
With the build error hopefully fixed:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=64b324f907fac990d592c7bb19b79a54cddc049c
Comment 31•8 years ago
|
||
Fails to build on Android:
/home/worker/workspace/build/src/gfx/2d/DrawTarget.cpp:79:50: error: 'ComputesRGBLuminanceMask_NEON' was not declared in this scope
Comment 32•8 years ago
|
||
Comment 33•8 years ago
|
||
Comment 34•8 years ago
|
||
Comment 35•8 years ago
|
||
Updated•8 years ago
|
Attachment #8872009 -
Attachment is patch: true
Comment 36•8 years ago
|
||
Pushed by jmuizelaar@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e12fa325e112
Add DrawTarget::IntoLuminance. r=jrmuizel,Bas
Comment 37•8 years ago
|
||
And then there's Windows 8: https://treeherder.mozilla.org/logviewer.html#?job_id=102713138&repo=mozilla-inbound
Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/75b68c6105e1
Comment 38•8 years ago
|
||
So I can reproduce bad rendering locally on tests like layout/reftests/w3c-css/submitted/masking/mask-composite-1c.html. Can you also reproduce this Mason? Was masking working for you before?
Flags: needinfo?(mchang)
Comment 39•7 years ago
|
||
I tried adding a Flush() and that didn't help.
Assignee | ||
Comment 40•7 years ago
|
||
This is happening because of [1]. If we have an ID2D1Image that isn't backed by an actual ID2D1Bitmap, you can't query for the bitmap from the image. Jeff and I talked about this over irc. We'll use push/pop layer instead. I got this working yesterday, but I'm hitting try failures now.
[1] http://searchfox.org/mozilla-central/source/gfx/2d/DrawTargetD2D1.cpp#355
Flags: needinfo?(mchang)
Assignee | ||
Comment 41•7 years ago
|
||
Carrying r+. Needed two fixes:
1) When pushing a layer, don't transform the rect.
2) A data source surface might be partially uploaded via an ID2D1Bitmap. In these cases, the sizes of the bitmap and the mask surface will be different.
Attachment #8872009 -
Attachment is obsolete: true
Attachment #8874881 -
Flags: review+
Assignee | ||
Comment 42•7 years ago
|
||
Successful tries:
D2D - https://treeherder.mozilla.org/#/jobs?repo=try&revision=34f27067adc4dafea6b7a1a0b4d14f36131c4ce1
All - https://treeherder.mozilla.org/#/jobs?repo=try&revision=07689ab9b6c7f8f67db2adfb2c8651d6ff77a639
Attachment #8874882 -
Flags: review?(jmuizelaar)
Updated•7 years ago
|
Attachment #8874882 -
Flags: review?(jmuizelaar) → review+
Comment 43•7 years ago
|
||
Pushed by mchang@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/f09499d7310c
Part 1 Add DrawTarget::IntoLuminance. r=jrmuizel
https://hg.mozilla.org/integration/mozilla-inbound/rev/eb0fe335fbe5
Part 2 Add D2D fuzzing for GPU Luminance. r=jrmuizel
Comment 44•7 years ago
|
||
Backed out for Android bustage while processing gfx/2d/moz.build:
https://hg.mozilla.org/integration/mozilla-inbound/rev/b6cae88b7d279dd14d8f52b71380b98a5fbb5786
https://hg.mozilla.org/integration/mozilla-inbound/rev/216d4d5cbb0c924b71616e7d721d99845962fb4f
Push with bustage: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=eb0fe335fbe5256fd2278f7c846699ffbbaecf83&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=runnable
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=104929019&repo=mozilla-inbound
task 2017-06-06T16:18:55.044362Z] 16:18:55 INFO - Reticulating splines...
[task 2017-06-06T16:18:56.276350Z] 16:18:56 INFO - Traceback (most recent call last):
[task 2017-06-06T16:18:56.279298Z] 16:18:56 INFO - File "/home/worker/workspace/build/src/configure.py", line 124, in <module>
[task 2017-06-06T16:18:56.279338Z] 16:18:56 INFO - sys.exit(main(sys.argv))
[task 2017-06-06T16:18:56.279369Z] 16:18:56 INFO - File "/home/worker/workspace/build/src/configure.py", line 34, in main
[task 2017-06-06T16:18:56.279395Z] 16:18:56 INFO - return config_status(config)
[task 2017-06-06T16:18:56.279432Z] 16:18:56 INFO - File "/home/worker/workspace/build/src/configure.py", line 119, in config_status
[task 2017-06-06T16:18:56.279461Z] 16:18:56 INFO - return config_status(args=[], **encode(sanitized_config, encoding))
[task 2017-06-06T16:18:56.279519Z] 16:18:56 INFO - File "/home/worker/workspace/build/src/python/mozbuild/mozbuild/config_status.py", line 147, in config_status
[task 2017-06-06T16:18:56.279551Z] 16:18:56 INFO - definitions = list(definitions)
[task 2017-06-06T16:18:56.279585Z] 16:18:56 INFO - File "/home/worker/workspace/build/src/python/mozbuild/mozbuild/frontend/emitter.py", line 183, in emit
[task 2017-06-06T16:18:56.279604Z] 16:18:56 INFO - objs = list(emitfn(out))
[task 2017-06-06T16:18:56.279635Z] 16:18:56 INFO - File "/home/worker/workspace/build/src/python/mozbuild/mozbuild/frontend/emitter.py", line 1026, in emit_from_context
[task 2017-06-06T16:18:56.279660Z] 16:18:56 INFO - for obj in self._handle_linkables(context, passthru, generated_files):
[task 2017-06-06T16:18:56.279690Z] 16:18:56 INFO - File "/home/worker/workspace/build/src/python/mozbuild/mozbuild/frontend/emitter.py", line 802, in _handle_linkables
[task 2017-06-06T16:18:56.279709Z] 16:18:56 INFO - 'exist: \'%s\'' % (symbol, full_path), context)
[task 2017-06-06T16:18:56.279728Z] 16:18:56 INFO - mozbuild.frontend.reader.SandboxValidationError:
[task 2017-06-06T16:18:56.279743Z] 16:18:56 INFO - ==============================
[task 2017-06-06T16:18:56.279763Z] 16:18:56 INFO - ERROR PROCESSING MOZBUILD FILE
[task 2017-06-06T16:18:56.279779Z] 16:18:56 INFO - ==============================
[task 2017-06-06T16:18:56.279806Z] 16:18:56 INFO - The error occurred while processing the following file or one of the files it includes:
[task 2017-06-06T16:18:56.279827Z] 16:18:56 INFO - /home/worker/workspace/build/src/gfx/2d/moz.build
[task 2017-06-06T16:18:56.279852Z] 16:18:56 INFO - The error occurred when validating the result of the execution. The reported error is:
[task 2017-06-06T16:18:56.279884Z] 16:18:56 INFO - File listed in SOURCES does not exist: '/home/worker/workspace/build/src/gfx/2d/LuminanceNEON.cpp'
Flags: needinfo?(mchang)
Assignee | ||
Comment 45•7 years ago
|
||
Fixed with android builds - https://treeherder.mozilla.org/#/jobs?repo=try&revision=098bb169608a2c17be282cfb2fada6fe3ff7bef5
Flags: needinfo?(mchang)
Comment 46•7 years ago
|
||
Pushed by mchang@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e14c50095f36
Part 1 - Add DrawTarget::IntoLuminance r=jrmuizel
https://hg.mozilla.org/integration/mozilla-inbound/rev/7fb451a4d22c
Part 2 - Add D2D fuzzing for IntoLuminance r=jrmuizel
Comment 47•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/e14c50095f36
https://hg.mozilla.org/mozilla-central/rev/7fb451a4d22c
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox55:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Comment 49•7 years ago
|
||
Also this commented line made it in:
+already_AddRefed<SourceSurface>
+DrawTargetD2D1::IntoLuminanceSource(LuminanceType aLuminanceType, float aOpacity)
+{
+ //return DrawTarget::IntoLuminanceSource(aLuminanceType, aOpacity);
+ if (aLuminanceType != LuminanceType::LUMINANCE) {
+ return DrawTarget::IntoLuminanceSource(aLuminanceType, aOpacity);
+ }
Comment 50•7 years ago
|
||
This hunk is also wrong:
+ if (StyleSVG()->mColorInterpolation ==
+ NS_STYLE_COLOR_INTERPOLATION_LINEARRGB) {
+ maskType = NS_STYLE_COLOR_INTERPOLATION_LINEARRGB;
+ }
+
RefPtr<SourceSurface> surface;
if (maskType == NS_STYLE_MASK_TYPE_LUMINANCE) {
- RefPtr<SourceSurface> maskSnapshot = maskDT->Snapshot();
+ RefPtr<SourceSurface> maskSnapshot =
+ maskDT->IntoLuminanceSource(GetLuminanceType(maskType),
+ aParams.opacity);
if (!maskSnapshot) {
return nullptr;
}
-
The change to NS_STYLE_COLOR_INTERPOLATION_LINEARRGB needs to be inside the 'if (maskType == NS_STYLE_MASK_TYPE_LUMINANCE)'
otherwise we use an ALPHA mask for NS_STYLE_COLOR_INTERPOLATION_LINEARRGB
Comment 51•7 years ago
|
||
Attachment #8875910 -
Flags: review?(mchang)
Assignee | ||
Comment 52•7 years ago
|
||
Comment on attachment 8875910 [details] [diff] [review]
Fix up mismerges
Review of attachment 8875910 [details] [diff] [review]:
-----------------------------------------------------------------
Thanks, sorry about that.
Attachment #8875910 -
Flags: review?(mchang) → review+
Assignee | ||
Updated•7 years ago
|
Flags: needinfo?(mchang)
Comment 53•7 years ago
|
||
Pushed by jmuizelaar@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/da58840d7f3d
Fixup DrawTarget::IntoLuminance mismerge. r=mchang
Comment 54•7 years ago
|
||
bugherder |
Updated•7 years ago
|
Depends on: 1429594
Updated•5 years ago
|
You need to log in
before you can comment on or make changes to this bug.
Description
•