Working on caching box shadows #802
Conversation
… to the texture callback function
…unction, replaced Rml::Mesh in cache with other properties
…rors on certain platforms
mikke89
left a comment
mikke89
left a comment
There was a problem hiding this comment.
Hey, I've finally gotten around to go through this PR now, and some testing with it.
First of all, thanks a lot for making the pull request. It's nice to see a focus on performance improvements too. I've made some comments throughout the PR, so please take a look at them.
I didn't measure it yet, but I am somewhat worried that all the extra data copies, hashing, and comparisons have some measurable performance impact. I have some suggestions in the comments that could alleviate some of that, but not all. Could you try to make some measurements, comparing it to master? Ideally, testing both cases: many unique box shadows, and another one for equivalent box shadows. An addition to the "Benchmarks" project would be very valuable here.
It would also be interesting to do the same for the geometry of all backgrounds/borders, as I suspect it is just as common to share styles in these cases. But that might as well be something to follow up with later.
…r to SVG cache, and hid away the box shadow stuff from the render manager, and ran clang format
|
Okay i tried all of the issues you mentioned, i'm still working on the cache part. However, I'm still trying to figure out how to deal with the shadow |
mikke89
left a comment
mikke89
left a comment
There was a problem hiding this comment.
Very nice progress.
I'm still trying to figure out how to deal with the shadow Geometry handle.
Yeah, I think it must be stored in the BoxShadowData, and then it needs to be retrieved from that data somehow. I understand it's a bit tricky with the other Geometry handles, I'll leave you to it :)
Include/RmlUi/Core/RenderManager.h
Outdated
| #include "RenderInterface.h" | ||
| #include "StableVector.h" | ||
| #include "Types.h" | ||
| #include "Utilities.h" |
There was a problem hiding this comment.
Some remnant includes and unnecessary changes now. I'm just making a note here to remember to revert these files that no longer have meaningful changes.
… solved code reviews
|
Finally got back to this, I think i implemented it correctly, I do need to fix the build issues, and merge the master branch back into this. |
|
Yeah nice, it's getting closer now, keep it up, and let me know when it's ready to take a new look at. |
…uction for box shadow cache
|
I finally got around to trying this, I installed vckpg so i could finally try out the visual tests, and turns out I think i did some copy paste error somewhere beacuse the box shadows are incorrectly cached |
|
I got it to work, i compared it to the visual tests and they are identical now, the tough part is profiling the performance difference now, I wasn't able to get the tracy profiler to work. Could you aid here? |
2 participants
PR to solve issue #799