Skip to content

Conversation

pauloamed
Copy link
Contributor

No description provided.


// these 2 sizes do NOT account margins
Math::Insets<Au> borders;
Math::Insets<Au> padding;
Copy link
Contributor Author

@pauloamed pauloamed Aug 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we be storing these values vs computing on the fly? discuss with the team

Au availableXSpace;
Au containingBlockX;
Au capmin;
Opt<Au> knownSizeX;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

benchmark this

@pauloamed pauloamed force-pushed the palm/rework-layout-calls branch from 890e084 to b5af72d Compare August 4, 2025 10:44
@pauloamed pauloamed force-pushed the palm/rework-layout-calls branch from b5af72d to d5e8d3d Compare August 6, 2025 09:05
@pauloamed pauloamed force-pushed the palm/rework-layout-calls branch from d5e8d3d to e5044d5 Compare August 6, 2025 09:43
@pauloamed pauloamed marked this pull request as ready for review August 6, 2025 09:44
@sleepy-monax sleepy-monax requested a review from Copilot August 6, 2025 09:59
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR refactors the layout system to improve how layout calls are handled, specifically focusing on separating content box and border box layout operations. The key changes include introducing a new UsedSpacings structure and reorganizing layout function responsibilities.

  • Introduces UsedSpacings struct to manage padding, borders, and margins consistently
  • Separates content box and border box layout operations into distinct functions
  • Updates table, flex, inline, and block layout contexts to use the new layout API
  • Adds comprehensive test cases for table padding resolution and flex/block intrinsic sizing

Reviewed Changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
src/vaev-engine/layout/layout.cpp Defines new UsedSpacings struct and exports for layout functions
src/vaev-engine/layout/layout-impl.cpp Major refactoring with new layout functions and spacing computation logic
src/vaev-engine/layout/table.cpp Updates table layout to use new border box layout functions and spacing system
src/vaev-engine/layout/replaced.cpp Simplifies replaced element layout by removing unnecessary function call
src/vaev-engine/layout/flex.cpp Refactors flex layout to use UsedSpacings and new layout API
src/vaev-engine/layout/inline.cpp Updates inline layout to compute spacings explicitly and use new layout functions
src/vaev-engine/layout/block.cpp Refactors block layout to use UsedSpacings and new child layout pattern
tests/css/display-table.xhtml Adds test case for table padding resolution behavior
tests/css/display-flex.xhtml Adds test case for flex intrinsic sizes with inline children
tests/css/display-block.xhtml Adds test case for intrinsic sizing across element trees
Comments suppressed due to low confidence (3)

src/vaev-engine/layout/layout-impl.cpp:11

  • Remove the empty line at the beginning of the struct scope.
import :layout.block;

src/vaev-engine/layout/layout-impl.cpp:286

  • Function name _adaptToContentBox uses underscore prefix which should be reserved for private methods, but this appears to be a helper function. Consider renaming to adaptToContentBox or making it properly private.
Input _adaptToContentBox(Input input, UsedSpacings const& usedSpacings) {

src/vaev-engine/layout/block.cpp:125

  • Function name _populateChildSpecifiedSizes uses underscore prefix which should be reserved for private methods, but this appears to be a helper function. Consider renaming to populateChildSpecifiedSizes or making it properly private.
void _populateChildSpecifiedSizes(Tree& tree, Box& child, Input& childInput, Au horizontalMargins, Opt<Au> blockInlineSize) {

@pauloamed pauloamed requested a review from sleepy-monax August 6, 2025 12:04
@pauloamed pauloamed force-pushed the palm/rework-layout-calls branch 2 times, most recently from 818433e to c2862da Compare August 7, 2025 13:31
@pauloamed pauloamed force-pushed the palm/rework-layout-calls branch from c2862da to bfd53f5 Compare August 7, 2025 14:00
@pauloamed pauloamed force-pushed the palm/rework-layout-calls branch from bfd53f5 to cd700dd Compare August 19, 2025 14:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants