Bug 504622 part 1. Rewrite fieldset border drawing to just clip to the area outside the legend instead of doing it in pieces with different clip rects. r=mattwoodrow,dbaron
authorBoris Zbarsky <bzbarsky@mit.edu>
Tue, 29 Nov 2016 15:52:30 -0500
changeset 324761 694d5dac1dccbe712f9b5ff8789ae525c738f03c
parent 324760 f4a2a329a7080cab9b7ebd1eb4b3295a1232b775
child 324762 096c7943a1c8f97370379463bde5195f0a4029e3
push id24
push usermaklebus@msu.edu
push dateTue, 20 Dec 2016 03:11:33 +0000
reviewersmattwoodrow, dbaron
bugs504622
milestone53.0a1
Bug 504622 part 1. Rewrite fieldset border drawing to just clip to the area outside the legend instead of doing it in pieces with different clip rects. r=mattwoodrow,dbaron This change will allow the border drawing code to deal with the following changes, which will make us no longer force the fieldset to be wider than the legend. Without this patch, allowing the fieldset to be narrower than the legend causes the vertical inline-start-side and inline-end-side borders of the fieldset to paint under the legend, because the current code only modifies the painting of the block-start-side border (the one the legend is positioned on). This does change behavior in one situation, which the new tests test. For relatively positioned legends, we used to use the original vertical location but the positioned horizontal location of the legend to decide which parts of the border to not paint. In the new setup, we use the original location for both. I did check that this new behavior matches Chrome and Safari. Edge seems to have our old behavior.
layout/forms/nsFieldSetFrame.cpp
layout/reftests/forms/fieldset/blue-1x1.png
layout/reftests/forms/fieldset/fieldset-border-image-1-ref.html
layout/reftests/forms/fieldset/fieldset-border-image-1a.html
layout/reftests/forms/fieldset/fieldset-border-image-1b.html
layout/reftests/forms/fieldset/fieldset-border-image-2-ref.html
layout/reftests/forms/fieldset/fieldset-border-image-2a.html
layout/reftests/forms/fieldset/fieldset-border-image-2b.html
layout/reftests/forms/fieldset/reftest.list
layout/reftests/forms/fieldset/relpos-legend-3-ref.html
layout/reftests/forms/fieldset/relpos-legend-3.html
layout/reftests/forms/fieldset/relpos-legend-4-ref.html
layout/reftests/forms/fieldset/relpos-legend-4.html
--- a/layout/forms/nsFieldSetFrame.cpp
+++ b/layout/forms/nsFieldSetFrame.cpp
@@ -105,16 +105,17 @@ public:
                        HitTestState* aState,
                        nsTArray<nsIFrame*> *aOutFrames) override;
   virtual void Paint(nsDisplayListBuilder* aBuilder,
                      nsRenderingContext* aCtx) override;
   virtual nsDisplayItemGeometry* AllocateGeometry(nsDisplayListBuilder* aBuilder) override;
   virtual void ComputeInvalidationRegion(nsDisplayListBuilder* aBuilder,
                                          const nsDisplayItemGeometry* aGeometry,
                                          nsRegion *aInvalidRegion) override;
+  virtual nsRect GetBounds(nsDisplayListBuilder* aBuilder, bool* aSnap) override;
   NS_DISPLAY_DECL_NAME("FieldSetBorderBackground", TYPE_FIELDSET_BORDER_BACKGROUND)
 };
 
 void nsDisplayFieldSetBorderBackground::HitTest(nsDisplayListBuilder* aBuilder, const nsRect& aRect,
                                                 HitTestState* aState, nsTArray<nsIFrame*> *aOutFrames)
 {
   // aPt is guaranteed to be in this item's bounds. We do the hit test based on the
   // frame bounds even though our background doesn't cover the whole frame.
@@ -150,16 +151,29 @@ nsDisplayFieldSetBorderBackground::Compu
       geometry->ShouldInvalidateToSyncDecodeImages()) {
     bool snap;
     aInvalidRegion->Or(*aInvalidRegion, GetBounds(aBuilder, &snap));
   }
 
   nsDisplayItem::ComputeInvalidationRegion(aBuilder, aGeometry, aInvalidRegion);
 }
 
+nsRect
+nsDisplayFieldSetBorderBackground::GetBounds(nsDisplayListBuilder* aBuilder,
+                                             bool* aSnap)
+{
+  // Just go ahead and claim our frame's overflow rect as the bounds, because we
+  // may have border-image-outset or other features that cause borders to extend
+  // outside the border rect.  We could try to duplicate all the complexity
+  // nsDisplayBorder has here, but keeping things in sync would be a pain, and
+  // this code is not typically performance-sensitive.
+  *aSnap = false;
+  return Frame()->GetVisualOverflowRectRelativeToSelf() + ToReferenceFrame();
+}
+
 void
 nsFieldSetFrame::BuildDisplayList(nsDisplayListBuilder*   aBuilder,
                                   const nsRect&           aDirtyRect,
                                   const nsDisplayListSet& aLists) {
   // Paint our background and border in a special way.
   // REVIEW: We don't really need to check frame emptiness here; if it's empty,
   // the background/border display item won't do anything, and if it isn't empty,
   // we need to paint the outline
@@ -212,85 +226,67 @@ nsFieldSetFrame::BuildDisplayList(nsDisp
 
 DrawResult
 nsFieldSetFrame::PaintBorder(
   nsDisplayListBuilder* aBuilder,
   nsRenderingContext& aRenderingContext,
   nsPoint aPt,
   const nsRect& aDirtyRect)
 {
-  // if the border is smaller than the legend. Move the border down
-  // to be centered on the legend.
+  // If the border is smaller than the legend, move the border down
+  // to be centered on the legend.  We call VisualBorderRectRelativeToSelf() to
+  // compute the border positioning.
   // FIXME: This means border-radius clamping is incorrect; we should
   // override nsIFrame::GetBorderRadii.
-  WritingMode wm = GetWritingMode();
-  nsRect rect = VisualBorderRectRelativeToSelf();
-  nscoord off = wm.IsVertical() ? rect.x : rect.y;
-  rect += aPt;
+  nsRect rect = VisualBorderRectRelativeToSelf() + aPt;
   nsPresContext* presContext = PresContext();
 
   PaintBorderFlags borderFlags = aBuilder->ShouldSyncDecodeImages()
                                ? PaintBorderFlags::SYNC_DECODE_IMAGES
                                : PaintBorderFlags();
 
   DrawResult result = DrawResult::SUCCESS;
 
   nsCSSRendering::PaintBoxShadowInner(presContext, aRenderingContext,
                                       this, rect);
 
   if (nsIFrame* legend = GetLegend()) {
-    Side legendSide = wm.PhysicalSide(eLogicalSideBStart);
-    nscoord legendBorderWidth =
-      StyleBorder()->GetComputedBorderWidth(legendSide);
-
-    // Use the rect of the legend frame, not mLegendRect, so we draw our
-    // border under the legend's inline-start and -end margins.
-    LogicalRect legendRect(wm, legend->GetRect() + aPt, rect.Size());
+    // We want to avoid drawing our border under the legend, so clip out the
+    // legend while drawing our border.  We don't want to use mLegendRect here,
+    // because we do want to draw our border under the legend's inline-start and
+    // -end margins.  And we use GetNormalRect(), not GetRect(), because we do
+    // not want relative positioning applied to the legend to change how our
+    // border looks.
+    nsRect legendRect = legend->GetNormalRect() + aPt;
 
-    // Compute clipRect using logical coordinates, so that the legend space
-    // will be clipped out of the appropriate physical side depending on mode.
-    LogicalRect clipRect = LogicalRect(wm, rect, rect.Size());
     DrawTarget* drawTarget = aRenderingContext.GetDrawTarget();
+    // We set up a clip path which has our rect clockwise and the legend rect
+    // counterclockwise, with FILL_WINDING as the fill rule.  That will allow us
+    // to paint within our rect but outside the legend rect.  For "our rect" we
+    // use our visual overflow rect (relative to ourselves, so it's not affected
+    // by transforms), because we can have borders sticking outside our border
+    // box (e.g. due to border-image-outset).
+    RefPtr<PathBuilder> pathBuilder =
+      drawTarget->CreatePathBuilder(FillRule::FILL_WINDING);
+    int32_t appUnitsPerDevPixel = presContext->AppUnitsPerDevPixel();
+    AppendRectToPath(pathBuilder,
+                     NSRectToSnappedRect(GetVisualOverflowRectRelativeToSelf() + aPt,
+                                         appUnitsPerDevPixel,
+                                         *drawTarget),
+                     true);
+    AppendRectToPath(pathBuilder,
+                     NSRectToSnappedRect(legendRect, appUnitsPerDevPixel,
+                                         *drawTarget),
+                     false);
+    RefPtr<Path> clipPath = pathBuilder->Finish();
+
     gfxContext* gfx = aRenderingContext.ThebesContext();
-    int32_t appUnitsPerDevPixel = presContext->AppUnitsPerDevPixel();
-
-    // draw inline-start portion of the block-start side of the border
-    clipRect.ISize(wm) = legendRect.IStart(wm) - clipRect.IStart(wm);
-    clipRect.BSize(wm) = legendBorderWidth;
 
     gfx->Save();
-    gfx->Clip(NSRectToSnappedRect(clipRect.GetPhysicalRect(wm, rect.Size()),
-                                  appUnitsPerDevPixel, *drawTarget));
-    result &=
-      nsCSSRendering::PaintBorder(presContext, aRenderingContext, this,
-                                  aDirtyRect, rect, mStyleContext, borderFlags);
-    gfx->Restore();
-
-    // draw inline-end portion of the block-start side of the border
-    clipRect = LogicalRect(wm, rect, rect.Size());
-    clipRect.ISize(wm) = clipRect.IEnd(wm) - legendRect.IEnd(wm);
-    clipRect.IStart(wm) = legendRect.IEnd(wm);
-    clipRect.BSize(wm) = legendBorderWidth;
-
-    gfx->Save();
-    gfx->Clip(NSRectToSnappedRect(clipRect.GetPhysicalRect(wm, rect.Size()),
-                                  appUnitsPerDevPixel, *drawTarget));
-    result &=
-      nsCSSRendering::PaintBorder(presContext, aRenderingContext, this,
-                                  aDirtyRect, rect, mStyleContext, borderFlags);
-    gfx->Restore();
-
-    // draw remainder of the border (omitting the block-start side)
-    clipRect = LogicalRect(wm, rect, rect.Size());
-    clipRect.BStart(wm) += legendBorderWidth;
-    clipRect.BSize(wm) = BSize(wm) - (off + legendBorderWidth);
-
-    gfx->Save();
-    gfx->Clip(NSRectToSnappedRect(clipRect.GetPhysicalRect(wm, rect.Size()),
-                                  appUnitsPerDevPixel, *drawTarget));
+    gfx->Clip(clipPath);
     result &=
       nsCSSRendering::PaintBorder(presContext, aRenderingContext, this,
                                   aDirtyRect, rect, mStyleContext, borderFlags);
     gfx->Restore();
   } else {
     result &=
       nsCSSRendering::PaintBorder(presContext, aRenderingContext, this,
                                   aDirtyRect, nsRect(aPt, mRect.Size()),
new file mode 100644
index e69de29bb2d1d6434b8b29ae775ad8c2e48c5391..5da01370015660ee34f6c45d4ce17366707177d2
GIT binary patch
literal 69
zc%17D@N?(olHy`uVBq!ia0vp^j3CUx1SBVv2j2ryJf1F&Asp9}6B2&#GcYnUGTdBs
RcQa6&!PC{xWt~$(69BT94`ToT
new file mode 100644
--- /dev/null
+++ b/layout/reftests/forms/fieldset/fieldset-border-image-1-ref.html
@@ -0,0 +1,13 @@
+<!DOCTYPE html>
+<style>
+  div {
+    padding: 0;
+    margin: 10px;
+    height: 100px;
+    width: 100px;
+    border: 10px solid;
+    border-image-source: url(blue-1x1.png);
+    border-image-outset: 10px;
+  }
+</style>
+<div></div>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/forms/fieldset/fieldset-border-image-1a.html
@@ -0,0 +1,13 @@
+<!DOCTYPE html>
+<style>
+  fieldset {
+    padding: 0;
+    margin: 10px;
+    height: 100px;
+    width: 100px;
+    border: 10px solid;
+    border-image-source: url(blue-1x1.png);
+    border-image-outset: 10px;
+  }
+</style>
+<fieldset></fieldset>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/forms/fieldset/fieldset-border-image-1b.html
@@ -0,0 +1,13 @@
+<!DOCTYPE html>
+<style>
+  fieldset {
+    padding: 0;
+    margin: 10px;
+    height: 100px;
+    width: 100px;
+    border: 10px solid;
+    border-image-source: url(blue-1x1.png);
+    border-image-outset: 10px;
+  }
+</style>
+<fieldset><legend></legend></fieldset>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/forms/fieldset/fieldset-border-image-2-ref.html
@@ -0,0 +1,14 @@
+<!DOCTYPE html>
+<style>
+  div {
+    padding: 0;
+    margin: 10px;
+    height: 100px;
+    width: 100px;
+    border: 10px solid;
+    border-image-source: url(blue-1x1.png);
+    border-image-outset: 10px;
+    transform: scale(0.5);
+  }
+</style>
+<div></div>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/forms/fieldset/fieldset-border-image-2a.html
@@ -0,0 +1,14 @@
+<!DOCTYPE html>
+<style>
+  fieldset {
+    padding: 0;
+    margin: 10px;
+    height: 100px;
+    width: 100px;
+    border: 10px solid;
+    border-image-source: url(blue-1x1.png);
+    border-image-outset: 10px;
+    transform: scale(0.5);
+  }
+</style>
+<fieldset></fieldset>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/forms/fieldset/fieldset-border-image-2b.html
@@ -0,0 +1,14 @@
+<!DOCTYPE html>
+<style>
+  fieldset {
+    padding: 0;
+    margin: 10px;
+    height: 100px;
+    width: 100px;
+    border: 10px solid;
+    border-image-source: url(blue-1x1.png);
+    border-image-outset: 10px;
+    transform: scale(0.5);
+  }
+</style>
+<fieldset><legend></legend></fieldset>
--- a/layout/reftests/forms/fieldset/reftest.list
+++ b/layout/reftests/forms/fieldset/reftest.list
@@ -3,14 +3,20 @@ fuzzy-if(skiaContent,2,13) == dynamic-le
 == fieldset-intrinsic-width-1.html fieldset-intrinsic-width-1-ref.html
 == fieldset-percentage-padding-1.html fieldset-percentage-padding-1-ref.html
 == fieldset-scroll-1.html fieldset-scroll-1-ref.html
 == fieldset-scrolled-1.html fieldset-scrolled-1-ref.html
 == fieldset-overflow-auto-1.html fieldset-overflow-auto-1-ref.html
 fuzzy-if(winWidget&&!layersGPUAccelerated,121,276) == positioned-container-1.html positioned-container-1-ref.html
 == relpos-legend-1.html relpos-legend-1-ref.html
 == relpos-legend-2.html relpos-legend-2-ref.html
+== relpos-legend-3.html relpos-legend-3-ref.html
+== relpos-legend-4.html relpos-legend-4-ref.html
 == sticky-legend-1.html sticky-legend-1-ref.html
 fuzzy-if(skiaContent,1,40768) == abs-pos-child-sizing.html abs-pos-child-sizing-ref.html
 == overflow-hidden.html overflow-hidden-ref.html
 == legend-rtl.html legend-rtl-ref.html
 == fieldset-grid-001.html fieldset-grid-001-ref.html
 == fieldset-flexbox-001.html fieldset-flexbox-001-ref.html
+== fieldset-border-image-1a.html fieldset-border-image-1-ref.html
+== fieldset-border-image-1b.html fieldset-border-image-1-ref.html
+== fieldset-border-image-2a.html fieldset-border-image-2-ref.html
+== fieldset-border-image-2b.html fieldset-border-image-2-ref.html
new file mode 100644
--- /dev/null
+++ b/layout/reftests/forms/fieldset/relpos-legend-3-ref.html
@@ -0,0 +1,8 @@
+<!DOCTYPE HTML>
+<html>
+<body>
+<fieldset>
+  <legend><div style="position:relative; left:20px">Legend</div></legend>
+</fieldset>
+</body>
+</html>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/forms/fieldset/relpos-legend-3.html
@@ -0,0 +1,8 @@
+<!DOCTYPE HTML>
+<html>
+<body>
+<fieldset>
+  <legend style="position:relative; left:20px">Legend</legend>
+</fieldset>
+</body>
+</html>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/forms/fieldset/relpos-legend-4-ref.html
@@ -0,0 +1,8 @@
+<!DOCTYPE HTML>
+<html>
+<body>
+<fieldset>
+  <legend><span style="position:relative; left:20px">Legend</span></legend>
+</fieldset>
+</body>
+</html>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/forms/fieldset/relpos-legend-4.html
@@ -0,0 +1,8 @@
+<!DOCTYPE HTML>
+<html>
+<body>
+<fieldset>
+  <legend style="display:inline; position:relative; left:20px">Legend</legend>
+</fieldset>
+</body>
+</html>