Proposal for replacing decorator syntax, add sprite sheet support #22
Comments
mikke89
added
enhancement
|
In Qfusion we have a special decorator called ninepatch, inspired by the Android UI primitive: https://github.com/Qfusion/qfusion/blob/master/source/ui/decorators/ui_ninepatch_decorator.cpp The goal was to allow atlas'ing of widget textures to improve performance. Perhaps you might want to look into bringing it into mainline |
|
It looks a lot like the tiled-box decorator, not sure I see the difference. Is it that you can choose whether to scale or wrap the (non-corner) borders? |
|
The point was to reduce the number of draw calls in the rendering pipeline originating in the UI. |
|
Ah, I see. I will certainly take a closer look at it. Batching geometry is one of the things I think can help out a lot, the number of draw calls quickly gets quite insane. I think I'll go ahead with these changes then, dropping custom properties completely, and use numeric IDs instead. |
Generally, I don't think that strings pose such a big problem. I vote for generality of the library. |
|
On top of that, I'm not really interested in porting the existing huge codebase built on top of the current libRocket to a new incompatible interface. I simply don't have time for that. |
|
The interface would mostly stay the same. You can still pass properties as string name/values if you prefer. The only major difference would be a new syntax for decorators and no longer the ability to add custom properties. And I might change the passed events from "resize" to EventId::Resize. I will of course properly consider the performance gains that are actually achieved before going this route, or changing the interface. |
|
Custom properties are also important and are considered basic DOM functionality nowadays, I think. |
|
I've never heard of custom properties in CSS, is this a thing? To be clear, element attributes will stay the same with arbitrary key-value pairs. |
Alright, that sounds better.
Custom CSS properties do exist though under the "CSS variable" monkier, please refer to https://developer.mozilla.org/en-US/docs/Web/CSS/var |
|
Ah, yes, CSS variables. They're quite different beasts from what we have now. We could support them just fine even with numeric property ids, not something I want to spend my time on though but it would be possible. |
|
I have an early implementation of this currently in the property_id branch. You can take a look at how the spritesheet and decorators look like if you'd like: |
|
What would be cool for the project publicity-wise is having on the frontpage some youtube videos showcasing library's features. At least, one wouldn't need to compile and launch individual demo projects to see what's going on there :P and yes, I'm talking about the spritesheet thing right now :P |
|
That's absolutely a good idea. I've been thinking that at the very least we should have more screenshots of the library up for display. And also, I'd like to see a 'showcase' sample with all the controls and most features displayed. Of course, videos are time-consuming though. :) Does this mean you like the spritesheet approach? :P Right now, I'm considering turning the performance branch into a v2.0 release once all the breaking changes I'd like to make are done. Then, any v2.x should have as few breaking changes as necessary (especially for the API). Spending a lot of my time now on publicity just before a breaking change might be better saved for later, but all contributions are welcome of course! ;) |
|
What I actually meant there is that CSS text and its effects can also be demonstrated in YT videos with sufficient quality these days. With short attention span people have nowadays, nobody is going to click on an external link to see how RCSS actually works. Sorta like this: https://www.youtube.com/watch?v=uhC-D4SdFis Although I have to agree that the spritesheet thing may not be the best canditate for such a video :P |
|
Ah, I see the appeal for this. I think rather than many videos like this on the frontpage, what I'd like to see is a single ~2 min video showcasing different aspects. We could do something like in your example, but a bit shorter. Writing code for various features, and seeing the results. |
|
Sorry for bringing this up again, but now that the focus has shifted from code refactoring and performance, would you consider porting our 9-patch decorator to the mainline? More on 9-patch: http://wiresareobsolete.com/2010/06/9-patches/ |
|
Yup, it's been on my todo-list :) Will get around to it soon. |
|
Just made an implementation of the ninepatch decorator: 7a86858. How do you like it? |
|
Not sure whether it's something wrong on my end or not but it's acting rather weird, spilling outside the element's box. Here's the RCSS: Here's how the hover decorator looks like:
Here's the box for the element in the Debugger:
|
|
Yeah, those coordinates look off. I'm assuming you want to stretch the inner part as marked:
Then, the coordinates will be (note the dimensions in the screenshot) button-orange-outer: 0px 0px 64px 64px; /* x y width height */
button-orange-inner: 7px 7px 50px 50px;That is, the outer image covers the whole thing, the inner rectangle is an inset inside the outer. I will of course document this better in time, but hope it makes sense for now :) Also note that the coordinates are in texture coordinates, so |
|
Yeah, I was somewhat guessing what 'outer' and 'inner' stand for in this context :) Here's how it looks now: The fact that the outer element is rendered at native resolution is somewhat disappointing as the original intention for the 9patch was higher DPI support. |
|
Here's how it's really supposed to look (excuse the pink color but it's the proportions that are important):
|
|
Oh, I see what you are trying to do with the dp units. We can make make it scale with dp, although my idea of high dpi support was to replace the spritesheet with a higher DPI version, so that we get the sharpest results. Maybe we can support both. Can you tell what values you used to specify the pink image (with how it worked before)? |
|
The old syntax was rather weird: Here's explanation of the syntax: |
|
Yeah, I'm not sure I get exactly what you are doing there. But I think that your center region is slightly expanded and drawn on top of the edges, does that sound right? We could add e.g. four 'feather' parameters that can do this for each edge, what do you think? |
|
Would be nice to have some better stretch modes, especially for the image decorator. If you ever have a go at that, pull requests are welcome ;) |
|
Btw, the following CSS code causes segmentation fault in the library: It appears that the corner tiles aren't configured properly but DecoratorTiledBox::Initialise has failed to catch that. |
Something like this maybe? |
|
So here, the center image is fixed size, and the other two are stretched? I was thinking in terms of these modes for the image decorator: Would that cover what you are trying to achieve? |
Yup.
I'm not sure I'm following you here. What I'm trying to achieve is to decorate a surface with an image, positioned at the center of the surface. Oh, another thing that occured to me, is that the new image decorator syntax doesn't allow easy rotation of elements, which is a crucial feature. |
That is exactly what the 'center' mode would do.
Was this possible before? I don't think I changed the shorthands for these types, but I may not remember exactly. |
Ah, I see now. Yes, having those modes would be awesome.
Yeah, the old syntax allowed specifying texture coordinates for the image, which in turn allowed rotation. The obvious use case is stuff like arrows, etc. Perhaps, this might work as the replacement? Qfusion/libRocket@0074062 Although this only works for tiled decorators and not for a plain image decorator. |
|
I see, I thought the flip/rotate modes were already available somehow, not sure how they were set before. I like your proposal there. I think I'd prefer a slightly different set of keywords though: The image decorator is a tiled decorator, so it should work as well. |
Well, it didn't for whatever reason. I'll tinker with it some more later today. |
I like the cw notation because it is very explicit about the direction of rotation. "rotate-xx" may seem ambiguous to some. |
|
I feel like 'rotate-' is easier to remember as a keyword, and it's also consistent with the 'flip' keywords. Also, rotation is always clockwise in CSS, plus you will spot it very fast if you do it the wrong way. I'm also a bit worried about starting with numbers as that may be interpreted by the number parser in the shorthand, not sure exactly. I don't think they're allowed in CSS. |
Alright, 'rotate' it is :P |
|
The reason rotation doesn't work for me on the image decorator is because when I'm using the shorthand syntax in the following manner: , the texture name is parsed as "/ui/baseui/gfx/controls/scrollbar_arrow," with the comma at the end. Update: figured I shouldn't have use the commas to separate the values, sorry for the confusion! |
|
No worries, I can see the confusion. The comma generally separates different tiles in the decorators. |
Should we open a separate issue for this? :P |
|
Yup, let's do it. Then I think we can close this issue. Do you want to have a go at implementing the stretch modes? :) |
|
Well, actually no, I don’t :D |
|
Alright, I'll have a go at it then ;) Yeah, I don't think those modes make much sense for anything other than the image decorator. We may want to do something like you did with the tiled vertical/horizontal/box as well, keeping the center fixed and scaling the edges. Are you just using this as a way to keep the center fixed, or have you found some use case for using images at the edges simultaneously? |
|
The only use case (more of a thought experiment actually) for stretched edges I’ve managed to come up with is some kind of gradient to smoothly fade out the center image |
|
Alright, I opened a new issue for the latest discussion. I think we can safely close this now :) |
|
Sorry for necromancing my way in here, but I'm at a complete loss. Is there a way to override a decorator's properties, e.g. image-src on hover? That wasn't a problem with Rocket's old verbose syntax, but I can't seem to find a way with the new syntax. |
|
@kim8823 No worries! :) The easiest approach is really to re-declare the whole decorator. It's a bit of an inconvenience with the new syntax. There is actually a little-documented feature where you can use the |
|
@mikke89 Well that's some good news, I was hoping for that. But I can't find anything in the docs, could you give me some pointers? I need to combine six images with three or possibly six classes that shave a varying number of texels off the edges, so I'm trying to avoid spelling all those combinations out. |
|
Did you try using sprites instead? Then you can declare these texel offsets separately. If you can, I'd strongly recommend that. Otherwise, if I remember correctly, this is the approach for deriving from another decorator: Say you have the following. @decorator stars : starfield {
num-layers: 5;
top-colour: #fffc;
bottom-colour: #fff3;
top-speed: 80.0;
bottom-speed: 20.0;
top-density: 8;
bottom-density: 20;
}You can replace Now you can make a variation of it by doing: @decorator stars_highspeed : stars {
top-speed: 500.0;
}Which inherits all the properties you did not override. And you use the two decorators like the following: decorator: stars;
/* or */
decorator: stars_highspeed;Writing this from memory, so I hope it works like that, let me know if you have any trouble :) |
|
Yes, that seems to be the correct syntax. Pretty awesome :) I've been looking at sprites, but I don't think I can separate the boxes from the images and combine them arbitrarily. Anyways, as it turns out, this all doesn't work in my case. The images are live video textures. The videos can change in size, but as far as RmlUI is concerned its always the same texture that's just updated somewhere else. So RmlUI doesn't know the texel-size and they only work in img tags... I'll tweak the texture coordinates myself in the Render Interface, there I know exactly which texture is which. Thanks for your help though! |
|
Cool :) No problem, glad it works out. You might want to also consider creating your own decorator, sounds a bit hacky to use the image decorators like this. But then again, as long as it works... :P |
While the work on increasing performance is well underway with great results already, there are some ideas that could improve it further but break some existing things, and possibly reduce some of the generality of the library.
Specifically, strings are passed and copied everywhere around in the code and need to be looked up in hash tables quite a lot. Especially for properties, events, and possibly pseudoclasses. What I propose instead is to use simple numeric IDs, one specific for each property. Everything would become much simpler then if we disallow custom property names (such as those used by decorators) and events.
But of course, we're not getting rid of decorators! Instead, I think we can improve them.
I think it is kind of awkward currently to have to specify the exact subrectangle in a texture right inside the decorator declaration. Instead, I propose first that we enable somehow to define a spritesheet with all subrectangles defining a given sprite in one place. Then we introduce a new property, either
decoratororbackground-image(as in css) which specifies a decorator by combining possibly multiple sprite names from the spritesheet, and comma-separated for multiple decorators.E.g. something like this:
One of the nice things is that we could then easily replace the whole spritesheet (1) to change to a different theme, or more importantly, (2) to specify a higher resolution image when the dp unit changes so that the textures are crisp at say 1dp==2px.
Thoughts?
The text was updated successfully, but these errors were encountered: