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.

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

default-font-size and default-font-family don't work in the Live-Preview #4298

Open
asuper0 opened this issue Jan 9, 2024 · 4 comments · May be fixed by #7011
Open

default-font-size and default-font-family don't work in the Live-Preview #4298

asuper0 opened this issue Jan 9, 2024 · 4 comments · May be fixed by #7011
Labels
a:live preview The Live preview of slintpad or the LSP (mT,bO) a:text Text rendering, fonts, Text input (mS,bF) bug Something isn't working

Comments

@asuper0
Copy link
Contributor

asuper0 commented Jan 9, 2024

System: Win10
VS code :1.83.0
Slint plugin:1.3.2

The default-font-size property just don't work in preview, but work when build and run.

@tronical tronical added bug Something isn't working a:text Text rendering, fonts, Text input (mS,bF) a:lsp slint-lsp and formating (mO,bT) labels Jan 9, 2024
@tronical
Copy link
Member

tronical commented Jan 9, 2024

Yes indeed, this also applies to the other default-font properties. When resolved at run-time, they resolve against the Window of the preview itself, not the Window in the user's Slint code :-(

I'm not sure how to solve that best yet...

@tronical
Copy link
Member

tronical commented Jan 9, 2024

We could change the signatures of say fn layout_info(..., window_adapter: &Rc<dyn WindowAdapter>) to something like fn layout_info(..., window_context: &WindowContext) and let WindowContext be a struct with a ref to the window adapter as well as the Pin<&WindowItem>. But besides the difficulty of implementing that, it also wouldn't help for render() itself, when we have to do the same resolution step inside the specific renderer and certainly don't know which WindowItem to pick.

Unless......... perhaps the other way of doing this is - at resolution time - to walk /up/ the item tree, starting at self_rc and then parent() until... we find the Window. That's not great, but it would be correct.

@ogoffart
Copy link
Member

ogoffart commented Jan 9, 2024

Another slightly related issue is that if one use default-font-size, it will not work when previewing sub components.
Perhaps default-font-size should be a "project" property (#267)? But we would still need to solve the problem on where to get the default font.

I guess the short term solution is indeed to walk up the item tree. But that is a fairly slow operation, we might not want to do that for every rendering of every Text. Adding some kind of WindowContext might not be a bad idea either.

@tronical tronical added a:builtin elements The runtime data structures related to the items (mO,bT) and removed a:lsp slint-lsp and formating (mO,bT) labels Jan 9, 2024
@tronical
Copy link
Member

tronical commented Jan 9, 2024

OIivier had a great idea for an optimisation here: We have to walk up the tree, but instead of traversing item by item we can traverse from component to component and just check if item at index 0 is a WindowItem. If it isn't, continue the walk.

@tronical tronical removed the a:builtin elements The runtime data structures related to the items (mO,bT) label Jan 15, 2024
@ogoffart ogoffart added the a:live preview The Live preview of slintpad or the LSP (mT,bO) label Nov 30, 2024
@ogoffart ogoffart changed the title default-font-size don't work in the vscode plugin "Slint Live-Preview" default-font-size and default-font-family don't work in the Live-Preview Nov 30, 2024
tronical added a commit that referenced this issue Dec 5, 2024
... by changing the resolution for the `WindowItem` to traverse the
item tree from the current item, instead of going to the window.

This doesn't quite fix #4298 because `rem` resolution is still missing.
That requires the built-in default font size function to be fixed as
well, which is non-trivial.

cc #4298
@tronical tronical linked a pull request Dec 5, 2024 that will close this issue
tronical added a commit that referenced this issue Dec 5, 2024
... by changing the resolution for the `WindowItem` to traverse the
item tree from the current item, instead of going to the window.

This doesn't quite fix #4298 because `rem` resolution is still missing.
That requires the built-in default font size function to be fixed as
well, which is non-trivial.

cc #4298
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a:live preview The Live preview of slintpad or the LSP (mT,bO) a:text Text rendering, fonts, Text input (mS,bF) bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants