raven
raven copied to clipboard
Load file into tabs
I made a start at implementing #64 and have promising results. Files now open into new tabs and the inspector displays whichever item you select.
The main changes were creating a new struct that goes inside AppState and stores all the data required for a given tab, namely the root object of that tab, the timeline scale ad the file path associated with that tab.
struct TabData {
// This holds the main timeline object.
// Pretty much everything drills into this one entry point.
otio::SerializableObject::Retainer<otio::SerializableObjectWithMetadata> root;
bool opened = true;
float scale = 100.0f; // zoom scale, measured in pixels per second
std::string file_path; // What file did we load?
};
Most of the rest of the changes are updating everything to work with the new data structure. There is now a GetActiveRoot() function that returns the root object of the active tab and should be used wherever you would have used appState.root. Actually implementing the tabs in the UI is fairly simple, I wrapped the existing Timeline draw code in a BeginTabBar section.
The main things left to do are handle closing tabs and opening the same file multiple times. More testing would also be good as there are undoubtably some bugs in there.
https://github.com/user-attachments/assets/ca0055c5-13d3-4cb0-a569-d64d2af0485a
I'm going to mark this as ready for review as I think it's mostly feature complete now. Aside from the ones I mentioned in the first post there are a few other changes:
- Only one copy of a file can be open at once in an instance of Raven. This is mainly due to tabs needing unique names but could be got around fairly easily if that is a feature that is needed?
- I added some functionality to
MainCleanupto make sure all the tabs are properly deleted on close - Added a function that specifically handles closing an individual tab
- Previously when Raven was opened an empty timeline was created but when I added tabs this felt redundant and clunky, there was always a empty tab floating around. Instead I've changed things so Raven opens with no timeline loaded and any timeline related options are greyed out until one is. Does this seem reasonable?
The latest commit includes a fix for #116 as I had to add it to make sure emscripten was working. I can move that to a seperate PR if that would be better
@ThomasWilshaw wow, this is great! I think putting that last fix in a separate PR would be nice. @jminor can you have a look?
@timlehr Thanks :) I'll make another PR for the Emscscripten stuff
This is mainly due to tabs needing unique names but could be got around fairly easily if that is a feature that is needed?
Seems reasonable to go ahead with that, and bracket the tabs with a PushId and PopId, that's a better practice in general in Imgui apps where element names can collide.
Thanks @meshula , I'll look into that. For my own reference here are the docs on push/pop ID
I've updated the tab code to use Push/PopID so it is now possible to open the same tab as many times as required. I've also opened a new PR for the Emscripten fix, should I undo those changes here or leave them as is?
@ThomasWilshaw if you rebase onto the #130 branch, this will merge nicely after merging the other one.
I think this is functionally complete now assuming the changes are approved. Below is how Raven now looks on opening and after loading a tab:
Is there anything else to add?
This looks great to me. @jminor, want to land this?
This looks great @ThomasWilshaw ! Thanks for making those changes.
In a follow-up lets add one more thing: When raven is launched with multiple filenames, it could open multiple tabs. (e.g. raven *.otio)
No problem, thanks for the review and feedback.
I'll try and have a look at that when I get chance.