The region specified for a mask (x/y/width/height) is ignored when rendering with Direct2D. See the attached test case. The mask region should limit the output to the green rectangle but instead is showing up to the red rectangle. With Direct2D disabled it works fine. Also looks fine in ESR10.

What kind of user effect are we talking about here? What is this a regression from?

(In reply to Lukas Blakk [:lsblakk] from comment #1) > What kind of user effect are we talking about here? What is this a > regression from? The bounds for the mask area are completely ignored. In the example test case, the area that should be hidden will be displayed. A bad result would be that important information that should be visible would be covered by the un-masked region making a site unuseable. A less severe result would simply be that graphics look horrible due to graphical elements showing where they should not. The regression is possibly from enabling the Azure-Thebes wrapper by default (bug 715768).

It would be great to get a regression window on this to see if its been affecting users for a while or not.

This is almost certainly a bug introduced by turning on Azure content, so I don't know that a regression window would be helpful.

Ah, thanks for clarifying that. At this point in the Firefox 15 cycle I don't see this being something to hold a release up, so not tracking for 15.

This testcase is currently broken on all renderers and platforms. Presumably this has something to do with DLBI? I do have a patch that I believe will fix this (it fixes some other similar testcases)

I raised bug 797507 for the problem with this test-case on all platforms. I believe this should fix the bug mentioned here, JWatt made several testcases for me which seem to work fine with this patch.

It would be good to have some helper API for this for when we start converting more code to use Azure directly. Maybe a RAII class where you can just do something like: AutoMaybeClipToSurface autoClipper(...args...); DT->Mask(...args...);

Comment on attachment 667612 [details] [diff] [review] Try to respect EXTEND_NONE when masking Review of attachment 667612 [details] [diff] [review]: ----------------------------------------------------------------- ::: gfx/thebes/gfxContext.cpp @@ +1403,5 @@ > cairo_mask(mCairo, pattern->CairoPattern()); > } else { > + bool needsClip = false; > + if (pattern->GetType() == gfxPattern::PATTERN_SURFACE && > + pattern->Extend() == gfxPattern::EXTEND_NONE) { This could use a high level description of how it's trying to solve the problem. @@ +1436,5 @@ > gfxPlatform::GetPlatform()->GetSourceSurfaceForSurface(mDT, surface); > > gfxPoint pt = surface->GetDeviceOffset(); > + > + mDT->PushClipRect(Rect(offset.x, offset.y, sourceSurf->GetSize().width, sourceSurf->GetSize().height)); Shouldn't the clip only happen with EXTEND_NONE?

Comment on attachment 667612 [details] [diff] [review] Try to respect EXTEND_NONE when masking Review of attachment 667612 [details] [diff] [review]: ----------------------------------------------------------------- ::: gfx/thebes/gfxContext.cpp @@ +1436,5 @@ > gfxPlatform::GetPlatform()->GetSourceSurfaceForSurface(mDT, surface); > > gfxPoint pt = surface->GetDeviceOffset(); > + > + mDT->PushClipRect(Rect(offset.x, offset.y, sourceSurf->GetSize().width, sourceSurf->GetSize().height)); Didn't you say cairo_mask_surfaces uses the default pattern? So that's always EXTEND_NONE, right?

(In reply to Bas Schouten (:bas.schouten) from comment #10) > > Didn't you say cairo_mask_surfaces uses the default pattern? So that's > always EXTEND_NONE, right? I believe so.

Backed out for reftest/crashtest failures. https://hg.mozilla.org/integration/mozilla-inbound/rev/c2e247f9d207 https://tbpl.mozilla.org/php/getParsedLog.php?id=17087035&tree=Mozilla-Inbound https://tbpl.mozilla.org/php/getParsedLog.php?id=17087183&tree=Mozilla-Inbound

Fixed a couple of small issues, carrying r+.

Comment on attachment 684510 [details] [diff] [review] Try to respect EXTEND_NONE when masking v2 Review of attachment 684510 [details] [diff] [review]: ----------------------------------------------------------------- Requesting review because there were some small API changes.

Comment on attachment 684510 [details] [diff] [review] Try to respect EXTEND_NONE when masking v2 Review of attachment 684510 [details] [diff] [review]: ----------------------------------------------------------------- ::: gfx/thebes/gfxContext.cpp @@ +1407,5 @@ > + // In this situation the mask will be fully transparent (i.e. nothing > + // will be drawn) outside of the bounds of the surface. We can support > + // that by clipping out drawing to that area. > + Rect surfaceSourceRect; > + if (!pattern->IsAzure() && When do we use non-azure patterns?

Comment on attachment 684510 [details] [diff] [review] Try to respect EXTEND_NONE when masking v2 Review of attachment 684510 [details] [diff] [review]: ----------------------------------------------------------------- Also, this should get a test. ::: gfx/thebes/gfxContext.cpp @@ +1407,5 @@ > + // In this situation the mask will be fully transparent (i.e. nothing > + // will be drawn) outside of the bounds of the surface. We can support > + // that by clipping out drawing to that area. > + Rect surfaceSourceRect; > + if (!pattern->IsAzure() && At minimum this should have a comment why.

I created a reftest for this as requested by Bas and pushed it (disabled for now): https://hg.mozilla.org/integration/mozilla-inbound/rev/5bb200c943cc Bas, you just need to uncomment the line in the reftest.list file in your push. Can you add the comment requested in comment 17 and push?

Added the requested comment and enabling the test jwatt made.

Any thoughts on this, Jeff?