The default bug view has changed. See this FAQ.

Draw rounded rects into mask layers without clipping

RESOLVED FIXED in mozilla15

Status

()

Core
Graphics: Layers
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: nrc, Assigned: nrc)

Tracking

15 Branch
mozilla15
Points:
---
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

5 years ago
ContainerState::SetupMaskLayer in FrameLayerBuilder.cpp currently uses clipping to draw rounded rects into the mask layer. We ought to be able to do better by drawing the rounded rects directly.
(Assignee)

Updated

5 years ago
Assignee: nobody → ncameron
(Assignee)

Comment 1

5 years ago
Created attachment 626265 [details] [diff] [review]
patch
Attachment #626265 - Flags: review?(roc)
Comment on attachment 626265 [details] [diff] [review]
patch

Review of attachment 626265 [details] [diff] [review]:
-----------------------------------------------------------------

::: layout/base/FrameLayerBuilder.cpp
@@ +2648,5 @@
> +  aEnd = NS_MIN<PRUint32>(aEnd, mRoundedClipRects.Length());
> +
> +  for (PRUint32 i = aBegin; i < aEnd; ++i) {
> +    RoundedRectTo(aContext, A2D, mRoundedClipRects[i]);
> +    aContext->Fill();

bogus!

::: layout/base/FrameLayerBuilder.h
@@ +376,5 @@
> +    // Draw (fill) the rounded rects in this clip to aContext
> +    void DrawRoundedRectsTo(gfxContext* aContext, PRInt32 A2D,
> +                            PRUint32 aBegin, PRUint32 aEnd) const;
> +    // 'Draw' (create as a path, does not stroke or fill) aRoundRect to aContext
> +    void RoundedRectTo(gfxContext* aContext, PRInt32 A2D,

AddRoundedRectPathTo
(Assignee)

Comment 3

5 years ago
Created attachment 626326 [details] [diff] [review]
patch
Attachment #626326 - Flags: review?(roc)
(Assignee)

Updated

5 years ago
Attachment #626265 - Attachment is obsolete: true
Attachment #626265 - Flags: review?(roc)
Comment on attachment 626326 [details] [diff] [review]
patch

Review of attachment 626326 [details] [diff] [review]:
-----------------------------------------------------------------

r+ with that

::: layout/base/FrameLayerBuilder.cpp
@@ +2658,5 @@
> +  }
> +
> +  // If there are more than one rounded rects, we need the intersection, so we must clip
> +  ApplyRoundedRectsTo(aContext, A2D, aBegin, aEnd);
> +  aContext->Paint();

It's actually slightly simpler (and potentially more efficient) to do
  ApplyRoundedRectsTo(aContext, A2D, aBegin, aEnd - 1);
  AddRoundedRectPathTo(aContext, A2D, mRoundedClipRects[aEnd - 1]);
  aContext->Fill();
Attachment #626326 - Flags: review?(roc) → review+
(Assignee)

Comment 5

5 years ago
Created attachment 626340 [details] [diff] [review]
patch

Reviewer's change and minor rebasing. Carrying r=roc.
Attachment #626326 - Attachment is obsolete: true
Attachment #626340 - Flags: review+
(Assignee)

Comment 6

5 years ago
Try push: https://tbpl.mozilla.org/?tree=Try&rev=a02fcb809219
(Assignee)

Updated

5 years ago
Keywords: checkin-needed
https://hg.mozilla.org/integration/mozilla-inbound/rev/602c1435026d
Flags: in-testsuite-
Keywords: checkin-needed
Target Milestone: --- → mozilla15
https://hg.mozilla.org/mozilla-central/rev/602c1435026d
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Comment on attachment 626340 [details] [diff] [review]
patch

Review of attachment 626340 [details] [diff] [review]:
-----------------------------------------------------------------

::: layout/base/FrameLayerBuilder.cpp
@@ +2668,5 @@
> +
> +  gfxRect clip = nsLayoutUtils::RectToGfxRect(aRoundRect.mRect, A2D);
> +  clip.Round();
> +  clip.Condition();
> +  // REVIEW: This might make clip empty.  Is that OK?

Was this comment meant to make it into the tree?
(Assignee)

Comment 10

5 years ago
(In reply to :Ms2ger from comment #9)
> Comment on attachment 626340 [details] [diff] [review]
> patch
> 
> Review of attachment 626340 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: layout/base/FrameLayerBuilder.cpp
> @@ +2668,5 @@
> > +
> > +  gfxRect clip = nsLayoutUtils::RectToGfxRect(aRoundRect.mRect, A2D);
> > +  clip.Round();
> > +  clip.Condition();
> > +  // REVIEW: This might make clip empty.  Is that OK?
> 
> Was this comment meant to make it into the tree?

That comment has been hanging around for ages, at least as long as I've been working on this code. I've just left it there when I've moved code around.
I think all comments marked "REVIEW:" can be removed. I know I've got quite a few ancient ones lying around...
(Assignee)

Comment 12

5 years ago
I'll remove it in my next relevant patch, probably for Bug 757347; doesn't seem worth filing a bug and/or making a separate patch just for this.
https://hg.mozilla.org/mozilla-central/rev/ec5d136e203d
https://hg.mozilla.org/mozilla-central/rev/5a327af385df
https://hg.mozilla.org/mozilla-central/rev/d5a60bfaa968
I presume the changesets in comment 13 were supposed to be for bug 757347 and just had the wrong bug #?
You need to log in before you can comment on or make changes to this bug.