New computed value system#188
Open
ananas-dev wants to merge 3 commits intomainfrom
Open
Conversation
sleepy-monax
requested changes
Apr 1, 2026
Comment on lines
+21
to
+28
| export struct Viewport { | ||
| // https://drafts.csswg.org/css-values/#small-viewport-size | ||
| RectAu small; | ||
| // https://drafts.csswg.org/css-values/#large-viewport-size | ||
| RectAu large = small; | ||
| // https://drafts.csswg.org/css-values/#dynamic-viewport-size | ||
| RectAu dynamic = small; | ||
| }; |
Member
There was a problem hiding this comment.
This is duplicated from Layout::Viewport
Comment on lines
43
to
49
|
|
||
| // FIXME: This should be renamed to changeDisplayArea, and we should make sure that | ||
| // its input is the page box and not the page area. | ||
| void changeViewport(Vec2Au viewport) { | ||
| if (_media.changeViewport(viewport)) | ||
| if (_media.changeDisplayArea(viewport)) | ||
| invalidateRender(); | ||
| } |
Member
There was a problem hiding this comment.
If we renamed it changeDisplayAreana() it should take something like:
using DisplayArea = Union<Viewport, PageArea>
Comment on lines
81
to
86
| Driver::RenderResult& ensureRender() { | ||
| if (_render) | ||
| return *_render; | ||
| _render = Driver::render(_document.upgrade(), _media, {.small = _media.viewportSize()}); | ||
| _render = Driver::render(_document.upgrade(), _media, {.small = _media.displayArea()}); | ||
| return *_render; | ||
| } |
Member
There was a problem hiding this comment.
Once we keep the display reana object and the full Viewport, we can pass it here instead of digging in media. Which will be needed for stuff for position: sticky;
Comment on lines
+120
to
+121
| context.media.width / Au{context.media.resolution}, | ||
| context.media.height / Au{context.media.resolution} |
Member
There was a problem hiding this comment.
I don't like that we transformed our resolution object from a nice and rich object to something semantically poor. This looks very confusing if you don't know/remeber that resolution actually means scale.
Comment on lines
+30
to
+55
| struct ComputationContext { | ||
| // FIXME: Fonts are not optional but are wrapped in Opt<> due to the lack of a default constructor. | ||
| // They also require mutability for caching stuff which forces ComputationContext to be passed | ||
| // by mut reference. Ideally they should be replaced by a richer version of | ||
| Opt<Gfx::Font> rootFont = NONE; | ||
| Opt<Gfx::Font> font = NONE; | ||
| WritingMode writingMode = WritingMode::HORIZONTAL_TB; | ||
| Viewport viewport = {.small = {800_au, 600_au}}; /// Viewport of the current box | ||
| Vec2Au displayArea = {800_au, 600_au}; | ||
| }; | ||
|
|
||
| export template <typename T> | ||
| concept Computible = requires(T const& a, ComputationContext& ctx) { | ||
| { toComputed(a, ctx) } -> Meta::Convertible<Computed<T>>; | ||
| }; | ||
|
|
||
| export template <typename T> | ||
| struct IdentityComputed { | ||
| using Type = T; | ||
| }; | ||
|
|
||
| export template <typename T> | ||
| requires Meta::Derive<_Computed<T>, IdentityComputed<T>> | ||
| Computed<T> toComputed(T const& val, ComputationContext&) { | ||
| return val; | ||
| } |
Member
There was a problem hiding this comment.
This needs to be consolidated into a Computable traits or something.
Exemple:
template <>
struct Computable<Foo> {
using Type = FooComputed;
static Type toComputed(Foo const& val, ctx){}
}
And for a simple case, you can just do
template <>
struct Computable<Foo> : IdentityComputable<Foo> {};
Comment on lines
+40
to
+47
| export template <> | ||
| struct _Computed<Ratio> { | ||
| using Type = Number; | ||
| }; | ||
|
|
||
| export Computed<Ratio> toComputed(Ratio const& val, [[maybe_unused]] ComputationContext& ctx) { | ||
| return val.num / val.deno; | ||
| } |
Member
There was a problem hiding this comment.
This should be identity computed since the computed value of a ratio is the pair of numbers
| .orientation = Print::Orientation::LANDSCAPE, | ||
|
|
||
| .resolution = Resolution::fromDpi(96), | ||
| .resolution = Resolution::fromDpi(96).toDppx(), |
Member
There was a problem hiding this comment.
This smell, see comment below and above 😅
| .orientation = Print::Orientation::LANDSCAPE, | ||
|
|
||
| .resolution = Resolution::fromDpi(96), | ||
| .resolution = Resolution::fromDpi(96).toDppx(), |
| .orientation = Print::orientationFromSize(viewport), | ||
|
|
||
| .resolution = Resolution::fromDpi(96), | ||
| .resolution = Resolution::fromDpi(96).toDppx(), |
Member
There was a problem hiding this comment.
Same smell as all the resolution stuff.
| }; | ||
|
|
||
| export template <typename T> | ||
| concept Computible = requires(T const& a, ComputationContext& ctx) { |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.