GPULayout Phase 1: 2-pass layout algorithm fundamental issues

[Pluglug]

Overview

During the Phase 2 code review, several fundamental issues were identified in the Phase 1 2-pass layout algorithm (estimate() → resolve()). These issues need to be addressed to ensure correct layout behavior.

Related: #115 (Phase 2 implementation)

---

Issues

P1-1: Width-dependent height problem

Location: layout.py lines 1611-1613, 1666-1667

Problem: element.calc_size(self.style) returns a "natural size" independent of width. However, actual height can depend on width (e.g., text wrapping). If width changes between estimate() and resolve(), the height is not recalculated.

Current code:

# In _estimate_vertical()
w, h = element.calc_size(self.style)
size = Size(w, h)

Impact: Text wrapping and other width-dependent content may have incorrect heights.

---

P1-2: scale_x inconsistency

Location: layout.py lines 1720-1732

Problem: scale_x is applied differently based on alignment:

• EXPAND: available_width * self.scale_x (can exceed parent bounds)
• Others: natural_width * self.scale_x

Current code:

if self.alignment == Alignment.EXPAND:
    element.width = available_width * self.scale_x  # Can exceed parent!
    element.x = cursor_x
else:
    natural_width, _ = element.calc_size(self.style)
    element.width = natural_width * self.scale_x

Impact: When scale_x > 1.0 with EXPAND alignment, items can overflow their parent container.

---

P1-3: scale_y double-scaling risk

Location:

• estimate(): lines 1616, 1670 (size.height * self.scale_y)
• resolve(): line 1735 (item_height * self.scale_y)

Problem: scale_y is applied in both estimate() and resolve(), creating a risk of double-scaling.

In estimate:

total_height += size.height * self.scale_y  # First application

In resolve:

element.height = item_height * self.scale_y  # Second application

Impact: Heights may be scaled twice if not carefully managed.

---

P1-4: Horizontal layout height constraint ignored

Location: layout.py line 1673

Problem: _estimate_horizontal() doesn't apply constraints.clamp_height() to the result.

Current code:

self.height = max_height + padding_y * 2  # No clamp!

Compare with vertical (line 1625):

self.height = constraints.clamp_height(total_height)  # Correctly clamped

Impact: Horizontal layouts don't respect height constraints.

---

P1-5: calc_height() ignores dirty state

Location: layout.py lines 1151-1152

Problem: calc_height() returns the cached self.height if it's > 0, without checking the _dirty flag.

Current code:

def calc_height(self) -> float:
    # estimate() 実行後なら計算済みの height を返す
    if self.height > 0:
        return self.height
    # ... fallback calculation

Impact: After modifications that set _dirty = True, calc_height() may return stale values.

---

Proposed Solutions

P1-1: Width-dependent height

• Add optional width parameter to calc_size()
• Or add a separate calc_height_for_width(width) method for items that need it

P1-2: scale_x inconsistency

• Decide on consistent behavior: either scale relative to natural size or constrain within parent
• Document the intended behavior

P1-3: scale_y double-scaling

• Apply scale_y only in one place (recommend resolve() only)
• estimate() should report natural sizes

P1-4: Horizontal height constraint

• Add constraints.clamp_height() call in _estimate_horizontal()

P1-5: calc_height dirty state

• Check _dirty flag before returning cached value
• Or deprecate this method in favor of layout() + self.height

---

Priority

These are foundational issues that affect all layouts. Recommend addressing before adding more features.

Issue	Severity	Complexity
P1-1	High	Medium
P1-2	Medium	Low
P1-3	High	Low
P1-4	Medium	Low
P1-5	Low	Low

---

Notes

• These issues were discovered during Phase 2 (align=True, alignment) code review
• Phase 2 implementation is complete but may be affected by these underlying issues
• Current workaround: use layout() consistently before accessing sizes