layout, render: fix positioning of absolute flex item descendants

Always placing boxes relative to their parent *seems* prettier, but the
implementation was broken and I can't come up with a sane working one.

So we're back to just special casing position: absolute in render.
Sad, but at least it works.  (I think it's also a bit more efficient.)

6 files changed, 40 insertions, 53 deletions

diff --git a/src/css/layout.nim b/src/css/layout.nim
index 0d94247f..de6e5720 100644
--- a/src/css/layout.nim
+++ b/src/css/layout.nim
@@ -1456,35 +1456,9 @@ proc pushPositioned(lctx: LayoutContext; box: CSSBox) =
     lctx.positioned[^1].stack
   else:
     stack
-  box.positioned = true
+  box.positioned = box.computed{"position"} != PositionStatic
   lctx.positioned.add(PositionedItem(stack: nextStack))
 
-# Offset the child by the offset of its actual parent to its nearest
-# positioned ancestor (`parent').
-# This is only necessary (for the respective axes) if either top,
-# bottom, left or right is specified.
-proc realignAbsolutePosition(child: BlockBox; parent: CSSBox;
-    dims: set[DimensionType]) =
-  var it {.cursor.} = child.parent
-  while it != parent:
-    let offset = if it of BlockBox:
-      BlockBox(it).state.offset
-    else:
-      InlineBox(it).state.startOffset
-    if dtHorizontal in dims:
-      child.state.offset.x -= offset.x
-    if dtVertical in dims:
-      child.state.offset.y -= offset.y
-    it = it.parent
-  if parent != nil and parent of InlineBox:
-    # The renderer does not adjust position for inline parents, so we
-    # must do it here.
-    let offset = InlineBox(parent).state.startOffset
-    if dtHorizontal in dims:
-      child.state.offset.x += offset.x
-    if dtVertical in dims:
-      child.state.offset.y += offset.y
-
 # size is the parent's size.
 # Note that parent may be nil.
 proc popPositioned(lctx: LayoutContext; parent: CSSBox; size: Size) =
@@ -1508,26 +1482,19 @@ proc popPositioned(lctx: LayoutContext; parent: CSSBox; size: Size) =
       # the available width, and we must re-layout.
       sizes.space.w = stretch(child.state.intr.w)
       lctx.layoutRootBlock(child, offset, sizes)
-    var dims: set[DimensionType] = {}
     if child.computed{"left"}.u != clAuto:
       child.state.offset.x = positioned.left + sizes.margin.left
-      dims.incl(dtHorizontal)
     elif child.computed{"right"}.u != clAuto:
       child.state.offset.x = size.w - positioned.right - child.state.size.w -
         sizes.margin.right
-      dims.incl(dtHorizontal)
     # margin.left is added in layoutRootBlock
     if child.computed{"top"}.u != clAuto:
       child.state.offset.y = positioned.top + sizes.margin.top
-      dims.incl(dtVertical)
     elif child.computed{"bottom"}.u != clAuto:
       child.state.offset.y = size.h - positioned.bottom - child.state.size.h -
         sizes.margin.bottom
-      dims.incl(dtVertical)
     else:
       child.state.offset.y += sizes.margin.top
-    if dims != {}:
-      child.realignAbsolutePosition(parent, dims)
   let stack = item.stack
   if stack.box == parent:
     stack.children.sort(proc(x, y: StackItem): int = cmp(x.index, y.index))
diff --git a/src/css/render.nim b/src/css/render.nim
index df679c91..2687a1cd 100644
--- a/src/css/render.nim
+++ b/src/css/render.nim
@@ -463,8 +463,8 @@ proc renderBlock(grid: var FlexibleGrid; state: var RenderState;
         else:
           grid.renderBlock(state, BlockBox(child), offset)
 
-# This function exists to support another insanity-inducing CSS
-# construct: negative z-index.
+# This function exists to support another insane CSS construct: negative
+# z-index.
 # The issue here is that their position depends on their parent, but the
 # parent box is very often not positioned yet.  So we brute-force our
 # way out of the problem by resolving the parent box's position here.
@@ -472,25 +472,31 @@ proc renderBlock(grid: var FlexibleGrid; state: var RenderState;
 # InlineBox offsets in the process - this means that there may be inline
 # boxes after this pass with an unresolved position which contain block
 # boxes with a resolved position.
-proc resolveBlockParent(box: CSSBox): BlockBox =
+proc resolveBlockOffset(box: CSSBox): Offset =
+  var dims: set[DimensionType] = {}
+  let absolute = box.positioned and
+    box.computed{"position"} in PositionAbsoluteFixed
+  if absolute:
+    if box.computed{"left"}.u != clAuto or box.computed{"right"}.u != clAuto:
+      dims.incl(dtHorizontal)
+    if box.computed{"top"}.u != clAuto or box.computed{"bottom"}.u != clAuto:
+      dims.incl(dtVertical)
   var it {.cursor.} = box.parent
   while it != nil:
     if it of BlockBox:
       break
     it = it.parent
   var toPosition: seq[BlockBox] = @[]
-  let findPositioned = box.positioned and
-    box.computed{"position"} in PositionAbsoluteFixed
   var it2 {.cursor.} = it
   var parent {.cursor.}: CSSBox = nil
   while it2 != nil:
+    if it2.render.positioned and (not absolute or it2.positioned):
+      break
     if it2 of BlockBox:
-      let it2 = BlockBox(it2)
-      if it2.render.positioned and (not findPositioned or it2.positioned):
-        break
-      toPosition.add(it2)
+      toPosition.add(BlockBox(it2))
     it2 = it2.parent
-  var offset = if it2 != nil: it2.render.offset else: offset(0, 0)
+  let absOffset = if it2 != nil: it2.render.offset else: offset(0, 0)
+  var offset = absOffset
   for i in countdown(toPosition.high, 0):
     let it = toPosition[i]
     offset += it.state.offset
@@ -501,6 +507,9 @@ proc resolveBlockParent(box: CSSBox): BlockBox =
     )
     it.inheritClipBox(parent)
     parent = it
+  for dim in DimensionType:
+    if dim in dims:
+      offset[dim] = absOffset[dim]
   if box of BlockBox:
     let box = BlockBox(box)
     box.render = BoxRenderState(
@@ -508,16 +517,12 @@ proc resolveBlockParent(box: CSSBox): BlockBox =
       offset: offset + box.state.offset,
       clipBox: DefaultClipBox
     )
-    if findPositioned:
-      box.inheritClipBox(it2)
-    else:
-      box.inheritClipBox(it)
-  return BlockBox(it)
+    box.inheritClipBox(if absolute: it2 else: it)
+  return offset
 
 proc renderPositioned(grid: var FlexibleGrid; state: var RenderState;
     box: CSSBox) =
-  let parent = box.resolveBlockParent()
-  let offset = if parent != nil: parent.render.offset else: offset(0, 0)
+  let offset = box.resolveBlockOffset()
   if box of BlockBox:
     grid.renderBlock(state, BlockBox(box), offset, pass2 = true)
   else:
diff --git a/test/layout/non-positioned-flex-item-respects-accepts-z-index.color.expected b/test/layout/non-positioned-flex-item-respects-accepts-z-index.color.expected
index 46d1979d..d1ea3a34 100644
--- a/test/layout/non-positioned-flex-item-respects-accepts-z-index.color.expected
+++ b/test/layout/non-positioned-flex-item-respects-accepts-z-index.color.expected
@@ -1 +1,4 @@
 test test
+test
+test test
+eh?
diff --git a/test/layout/non-positioned-flex-item-respects-accepts-z-index.html b/test/layout/non-positioned-flex-item-respects-accepts-z-index.html
index 956f2591..fabad52f 100644
--- a/test/layout/non-positioned-flex-item-respects-accepts-z-index.html
+++ b/test/layout/non-positioned-flex-item-respects-accepts-z-index.html
@@ -1,5 +1,17 @@
 <!DOCTYPE html>
+<div style="position: relative; line-height: 1em">
 <div style="display: flex">
 <div style="background: red; position: absolute; height: 1em; width: 5ch"></div>
 <div style="z-index: 1">
 test test
+</div>
+</div>
+<div style="display: flex; flex-direction: column">
+<div>test</div>
+<div style="z-index: -1">
+test test
+<div style="position: absolute; top: 3em">
+eh?
+</div>
+</div>
+</div>
diff --git a/test/layout/position-absolute-with-relative-inline-parent-offset.color.expected b/test/layout/position-absolute-with-relative-inline-parent-offset.color.expected
index fd207776..8feac12e 100644
--- a/test/layout/position-absolute-with-relative-inline-parent-offset.color.expected
+++ b/test/layout/position-absolute-with-relative-inline-parent-offset.color.expected
@@ -1,6 +1,6 @@
 
 Note: per CSS, the absolute box's sizing is explicitly UB, apparently to
-accomodate for Gecko's broken rendering. We follow Blink.
+accommodate for Gecko's broken rendering. We follow Blink.
 
 
 
diff --git a/test/layout/position-absolute-with-relative-inline-parent-offset.html b/test/layout/position-absolute-with-relative-inline-parent-offset.html
index 88f5882f..ddabca34 100644
--- a/test/layout/position-absolute-with-relative-inline-parent-offset.html
+++ b/test/layout/position-absolute-with-relative-inline-parent-offset.html
@@ -2,7 +2,7 @@
 <!-- don't ask -->
 <p>
 Note: per CSS, the absolute box's sizing is explicitly UB, apparently to
-accomodate for Gecko's broken rendering.  We follow Blink.
+accommodate for Gecko's broken rendering.  We follow Blink.
 </p>
 <br>
 <br>test test <span style="position:relative">test test<div style="display: inline-block">adsfasdf</div><BR>