-
Notifications
You must be signed in to change notification settings - Fork 25
Page base size #80
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Page base size #80
Conversation
5a8409c
to
8366024
Compare
export struct Computer { | ||
Media _media; | ||
Media const _media; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i think this is needed because after build()
, the class has internal structures dependent on the media value. so having this attribute media
not const allows us to set an invalid state on Computer
Guten Morgen @sleepy-monax I don't know how to set copilot to review this beautiful PR so we waste less neurons here. |
There was a problem hiding this 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 implements CSS Page Size functionality for the Vaev engine, enabling proper handling of page dimensions in print layouts. The implementation adds support for the CSS size
property as defined in the CSS Page Module Level 3 specification.
Key changes:
- Added page size parsing and resolution logic with support for predefined paper sizes and custom dimensions
- Refactored the style system to remove separate
ComputedValues
and consolidate font handling withinSpecifiedValues
- Implemented page bounds calculation considering margins and orientation
Reviewed Changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 4 comments.
Show a summary per file
File | Description |
---|---|
src/vaev-engine/values/page.cpp | Added page size parsing with support for paper stock types and custom dimensions |
src/vaev-engine/values/length.cpp | Added absolute length resolution function for converting CSS units to application units |
src/vaev-engine/style/specified.cpp | Added page size property to style values and integrated font face handling |
src/vaev-engine/style/props.cpp | Implemented PageSizeProp parser for CSS size property |
src/vaev-engine/style/computer.cpp | Added page base size computation and refactored font handling |
src/vaev-engine/layout/values.cpp | Refactored length resolution to use centralized absolute length conversion |
src/vaev-engine/layout/builder.cpp | Updated box construction to remove separate font face parameter |
src/vaev-engine/layout/base.cpp | Simplified Box structure by removing separate font face member |
src/vaev-engine/driver/print.cpp | Implemented page bounds calculation and media size updates |
src/vaev-engine/dom/element.cpp | Removed computed values structure and methods |
Comments suppressed due to low confidence (1)
Related PR: skift-org/karm#6 |
3b7f4ae
to
e6ea763
Compare
""s, | ||
(f64)pageRect.size().width * media.resolution.toDppx(), | ||
(f64)pageRect.size().height * media.resolution.toDppx() | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we try to name these pages?
e6ea763
to
7b9da24
Compare
No description provided.