Skip to content
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.

Sign up for GitHub

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

Jump to bottom

xilem_web svg regression: Missing stroke attribute #716

Closed
burrbull opened this issue Oct 26, 2024 · 2 comments · Fixed by #717
Closed

xilem_web svg regression: Missing stroke attribute #716

burrbull opened this issue Oct 26, 2024 · 2 comments · Fixed by #717

Comments

@burrbull
Copy link

burrbull commented Oct 26, 2024 •
edited
Loading

Yet another regression after #699

Circle::new((0.0, 0.0), 5.0)
            .fill(Color::rgb8(0x99, 0x99, 0x99))
            .stroke(Color::BLACK, Stroke::new(0.5))

Before:
Screenshot_20241026_132228

After:
Screenshot_20241026_132513
@Philipp-M Philipp-M mentioned this issue Oct 26, 2024
Merged
@Philipp-M
Copy link
Contributor

Thanks for catching this, can you check if #717 fixes this?

@burrbull
Copy link
Author

Seems like yes.

github-merge-queue bot pushed a commit that referenced this issue Oct 28, 2024
@Philipp-M
xilem_web: Add a few boolean attribute modifiers for `HtmlInputElemen…
f32277a 
…t` and factor element state flags out of these modifiers into `ElementFlags` (#717)

This PR starts with supporting specialized elements (i.e. more than just
the `Element` DOM interface), starting with boolean attributes in
`HtmlInputElement` (`checked`, `default_checked`, `disabled`,
`required`, `multiple`).
With a general `OverwriteBool` modifier which is optimized by encoding
boolean flags into a `u32`, thus not requiring allocations (compared to
the other modifiers we currently have), but is limited to max 32
overwrites, which I don't think should be a real-world limitation (i.e.
`input_el.checked().checked()...checked()` 32 times makes no sense), I
also started adding tests for this, as it juggles around with
bit-operations (and we should generally start writing more tests).
I have originally planned to feature-flag this (i.e. have a feature like
`HtmlInputElement`).
But I'd like to see how far we can go without this, I haven't yet
noticed significant (binary-size) regressions in the todomvc example
(which uses `input` elements) that justifies the worse DX that
additional features introduce.
Having features for this is also not optimal for another reason: It
changes the API (e.g. `DomNode::Props` is `props::HtmlInputElement`
instead of `props::Element`.

It also factors the element state flags (`was_created`, `in_hydration`)
into a separate `ElementFlags` which is shared within a `Modifier<M>`
struct (similar as `PodMut` or `WidgetMut` in masonry), so that we don't
have to duplicate that state in every modifier. Additionally a new flag
`needs_update` is introduced, which indicates that the element in
general needs to update any modifier, and is entirely an optimization to
avoid checking every modifier whether it has changed (not yet that
important, but when we have a lot of modifiers per element, having to
check every modifier is less efficient, it's also already *slightly*
visible in the js-framework-benchmark).
For this, it unfortunately suffers similar as #705 from the free-form
syntax by being a little bit more verbose (which may be reverted after
`arbitrary_self_types` are stable).

It should also fix #716
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
2 participants
@burrbull @Philipp-M