Hot reloading style sheets

[lut0pia]

Hello,

I'm currently integrating RmlUi 3.3 to my engine and wanted to try and reload style sheets when they change on disk to have the results directly appear on screen. I've seen ElementDocument::SetStyleSheet so I suppose updating the style sheets themselves at runtime is not a problem. I noticed the StyleSheetFactory has a cache of already parsed style sheets and a method to clear that cache, which seems to go my way, but I have not found any way to ask a document to reload its style sheets, or to clear the style sheet cache for a specific file (or combination with said file). I would gladly clear the whole cache (performance isn't really an issue as it's a dev feature), and then use the factory to recreate the style sheet(s) for my document and set it, but I haven't found a way to know which sources a StyleSheet uses.

Am I missing something or is that a current limitation? If there's anything to do to make it happen I'm willing to work on a PR, it doesn't seem to me (if SetStyleSheet updates everything as I suspect) like a whole lot of work, there's only a bit of information missing from the client's perspective.

I'm using 3.3 because it's the latest stable version but I will switch as soon as a new version comes out if that helps. I'm very interested in hot reloading as it would greatly help the UI artist's workflow.

Thank you for your time, I'm eager to continue working with this lib :)

[mikke89]

Hi, and welcome!

Generally, I'd say you have a good understanding of the situation. What I've done is just to reload the whole document (after clearing the cache), but I agree that just updating the style sheet when it changes would be very convenient in many situations. I would very much be open to a PR.

So, right now, all the style sheet information, both external and inline are just merged into a single style sheet. We don't really store the origins in an easily accessible manner currently. I'm not sure how best to solve it, because we will have to pull in all external and inline style sheets again and merge them together. I'm thinking perhaps one of these options:

1. Add a reference back to the cache from ElementDocument. User tells cache a given file has been updated, and calls some new ElementDocument::ReloadStyleSheets function. However, what if the cache is cleared? Then the style sheets are essentially lost, and it is impossible to recreate.
2. Add a function that will re-parse the document header from the original RML source path, and reload all referenced style sheets. This should also work easily for reloading inline style sheets. Probably requires some more code, but the solution should be more robust, and we could potentially expand it to reload scripts as well.
3. So the document's style sheet indirectly contains the source paths through Property::source.path. By walking through all the StyleSheetNodes, we could erase all properties originating from the changed style sheet, load the new style sheet, and merge it with the document's style sheet. To me, this approach sounds very fragile. However, it could be an approach to get the list of dependent style sheet files if we want avoid storing a list on each document.

Out of these, I'm favoring number 2. However, I might be overthinking this right now. Let me know if any of this makes sense :)

[lut0pia]

Hey thanks for responding so quickly!

I figured reloading the whole document would work in some cases but I suppose if the game modifies content on the fly (which seems likely) then it would not retain any of that content :/

So I've dived into the code and I have a couple of remarks/questions:

1. I had not realized that the StyleSheetFactory was not accessible from client code, which seems problematic if we want the client to clear the cache, is there something I'm missing? What code should the client call to clear the cache? Ideally for a single source (instead of the whole cache).

   1. Going further, we could actually enhance the SystemInterface with a way to directory watch or compare file timestamps to clear parts of the cache automatically, but this seems out of scope right now, which is why I assume the client will have to do that work.

2. It seems like the StringList version of the StyleSheetFactory::GetStyleSheet function has become dead code between version 3.3 and master, since now all merging seems to be done directly inside ElementDocument::ProcessHeader. That sounds actually good from my perspective since it means we don't have to deal with a cache of combined sheets. Are there plans to remove the code entirely or do we have to support it in the future?

   1. Also I noticed the way combined_key was generated could cause unwanted collisions (["foo.rcss", "bar.rcss"] would have the same key as ["foobar.rcss"]). I was about to test a fix when I realized the code was no longer used.

3. About your option 1: The back reference seems good to me, especially to automate the rebuilding, but I don't understand how ReloadStyleSheets would know what to do here, we are still missing critical information about which style sheets to load (and about inline RCSS), aren't we? I also don't understand what you said about style sheets getting lost.

4. About your option 2: I think generally I also lean towards this option, however I don't think we should have to reread/reparse the document for our purposes. Here are the downsides I'm seeing:

   1. We would be reparsing a file for information we could have saved, that is already partially saved, and effectively we would want that same information.

   2. We would be reading a file but updating the UI with only part of its content. What I mean is that if the user edits both some inline RCSS and the body of a document, they would only see the modifications made to the inline RCSS while the document would still have the previous structure. I think that would be counterintuitive and it would be preferable to only refresh external resources (but I understand that's a matter of opinion, I may be biased by the fact that I would rather use external RCSS files than inline RCSS in general).

5. About your option 3: It does seem risky, and having not much experience with RmlUi I also wouldn't know where to begin doing such a thing!

6. Here's how I would go about it:

   1. Allow the client to clear (part of) the stylesheet cache somehow (cf 1.).

   2. Create an array similar to a ResourceList in ElementDocument that would have the parsed inline StyleSheet instead of the content field. This array would be filled during ElementDocument::ProcessHeader.

      1. Another simpler solution here would be to keep the RCSS ResourceList itself but that would mean keeping unparsed RCSS which does not seem ideal.

   3. Create ElementDocument::ReloadStyleSheet which would use the aforementioned array and the StyleSheetFactory to merge everything again, inline and external, in the same order. This method by itself would not clear the cache but it would be a way to rebuild the style sheet (maybe it could be called RebuildStyleSheet) if the cache has been cleared.

   4. Create a back reference from the stylesheet cache to documents, as you described in your option 1 (however I do not know how to safely keep references to documents).

   5. Call the new reload/rebuild method on referenced documents when a cache entry gets cleared.

   What do you think? I guess the heart of the issue is whether or not we are willing to keep an array of source paths and parsed inline stylesheets inside of ElementDocument that would mostly be for development purposes.

P.S.: I am truly sorry about the absolute novel I just wrote.

[mikke89]

Appreciate the detailed response. Don't worry about the novel, you show a lot of skill here and I find it all productive :)

1. The client can call Rml::ClearStyleSheetCache() in Factory.h to clear the whole cache. We can't clear single files right now, and I don't really see an important use case for that. This should only be used during development, and taking a few ms extra each time we manually save a source file shouldn't be a particularly important concern in my opinion.

   1. While that would be a nice feature, I think such code belongs on the client side. Including this in the sample projects would be really nice though.

2. You're right. There has been some changes here very recently. The problem with this was that it did not properly consider the order dependency between inline and external style sheets. I think we might as well remove this, although in some cases it might result in slightly slower loading (due to the extra merging). We would have to re-think this anyway, also for the reason you mention, so it should be safe to remove.

3. What I meant to say here is, if the client clears the cache, then any back-reference to the cache would also be invalidated, so we wouldn't be able to reconstruct the style sheets. So yes, as you say we would have to store source paths and possibly inline style sheets with this approach.

4-6.
I think it is important to state here that the parsing and style sheet building step takes in the order of milliseconds to complete, and here we are talking about a development-only impact when reloading style sheets, which, after all, would only happen on rare occasions and when users/authors save a file. I wouldn't be concerned with the wait-time here, instead, I would be slightly more concerned about storing extra data that would not be needed in production code.

In my own case, I like to store document-specific styles in inline RCSS, so I would find it highly useful if we could reload inline styles as well, without affecting the DOM tree / reloading the whole document. As you say, the latter is a more destructive operation. In the case where a user saves an RML file, they would have to make a choice: Either reload the document entirely, or reload the the style sheet only. In the case of hot reloading, the latter could be done automatically when saving the file, while the former could be done manually. But in any case, this decision would be left to the client.

So I guess that is where our priorities differ a bit, it wouldn't personally be as useful to me if I couldn't reload inline style sheets. I'm also a bit worried about storing a lot of data that would only be useful during development. Especially the inline style sheets, they can easily become a sizable amount of data. I'm fine with a list of external resources though.

So, my preferred approach would be a method that re-parses the header. This way we would avoid storing the inline style sheets, and we can easily support reloading inline sources as well.

[lut0pia]

To be clear my reluctance about reparsing has nothing to do with performance and more to do with structure/philosophy. However I completely understand your points about reloading inline style and the problem of keeping dev things in memory, so let's head the reparsing way!

So I started by taking another look at the callstack when getting to ElementDocument::ProcessHeader and it seemed pretty hard to decorellate parsing and the entire document loading process, anything that came to mind seemed pretty ugly/hacky. I also didn't know where to put the new function, I wanted it inside ElementDocument by default but it didn't seem to align with the current architecture of RmlUi where streams are opened by the context, then the factory actually creates the parser, etc.

It eventually came to mind that I wanted to essentially do LoadDocument but only cared about the style sheet result, which is when it struck me (and quite frankly I feel stupid for not thinking about this earlier) that I could just do this:

Factory::ClearStyleSheetCache();
ElementDocument* tmp_doc = GetContext(0)->LoadDocument(document->GetSourceURL());
document->SetStyleSheet(tmp_doc->GetStyleSheet());
GetContext(0)->UnloadDocument(tmp_doc);

Now this is kind of a bazooka for a (fairly large) fly but since it's a dev feature I could actually use it without feeling bad about it 🤷‍♀

I've been testing it in the demo sample (refresh every second to avoid dealing with directory watching) and it works like a charm! I was able to seamlessly change inline or external style and it would refresh within the second, without losing any state information.

Besides it having relatively bad performance, do you know of any problems that could arise with this solution? Otherwise I think I'm going to go with this for my engine.

P.S.: Since it is small and kind of hacky I didn't think you'd want to have that in RmlUi at first but if you do I guess this could be put inside a ElementDocument::ReloadStyleSheet method easily, so tell me if you want me to make a PR with that.

[mikke89]

That's actually a pretty creative solution :) I don't really see a reason it shouldn't work, perhaps it may interfere a bit with unexpected client callbacks. And of course it's pretty slow, but it should not be a high priority case.

I do see your point that a more dedicated solution would be a bit invasive, we would probably have to modify the XML parser to enable an early exit.

We could take this general approach, but make it faster and with less side-effects. Instead of Context->LoadDocument, we could just call Factory::InstanceDocumentStream and retrieve the style sheet from that. Then, at least the layouting step and some of the callbacks will be avoided.

I'd very much be happy with such a function on ElementDocument if you'd like to do a PR. Then, if we feel like it, we could always optimize it at a later stage.

[lut0pia]

I created PR #159 with the new method as discussed, I don't know if I have to link the issue to it myself? In which case I did not succeed as you can see in the history of the PR :D