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.

Sign up for GitHub

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

Timeline arrow key navigation #89

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Timeline arrow key navigation #89

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
12ebf08
Timeline arrow key navigation
ThomasWilshaw Nov 29, 2024
adb2a3f
timeline: Add up arrow support
ThomasWilshaw Dec 2, 2024
0d5a0a5
timeline: wrap keyboard commands in IsWindowFocused()
ThomasWilshaw Dec 2, 2024
f52b918
Video up and Audio down use same logic
ThomasWilshaw Dec 2, 2024
7665ea0
Improve logic and add down arrow support
ThomasWilshaw Dec 2, 2024
63a6fb1
Combine up and down arrow code
ThomasWilshaw Dec 2, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions app.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,8 @@ struct AppState {
float track_height = 30.0f; // current track height (pixels)
otio::RationalTime playhead;
bool scroll_to_playhead = false; // temporary flag, only true until next frame
bool scroll_left = false; // temporary flag, only true until next frame
bool scroll_right = false; // temporary flag, only true until next frame
otio::TimeRange
playhead_limit; // min/max limit for moving the playhead, auto-calculated
float zebra_factor = 0.1; // opacity of the per-frame zebra stripes
Expand Down
2 changes: 1 addition & 1 deletion main_glfw.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ int main(int argc, char** argv)
ImGui::CreateContext();
ImPlot::CreateContext();
ImGuiIO& io = ImGui::GetIO(); (void)io;
io.ConfigFlags |= ImGuiConfigFlags_NavEnableKeyboard; // Enable Keyboard Controls
//io.ConfigFlags |= ImGuiConfigFlags_NavEnableKeyboard; // Enable Keyboard Controls
//io.ConfigFlags |= ImGuiConfigFlags_NavEnableGamepad; // Enable Gamepad Controls
io.ConfigFlags |= ImGuiConfigFlags_DockingEnable; // Enable Docking
io.ConfigFlags |= ImGuiConfigFlags_ViewportsEnable; // Enable Multi-Viewport / Platform Windows
Expand Down
2 changes: 1 addition & 1 deletion main_win32.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ int main(int argc, char** argv)
IMGUI_CHECKVERSION();
ImGui::CreateContext();
ImGuiIO& io = ImGui::GetIO(); (void)io;
io.ConfigFlags |= ImGuiConfigFlags_NavEnableKeyboard; // Enable Keyboard Controls
//io.ConfigFlags |= ImGuiConfigFlags_NavEnableKeyboard; // Enable Keyboard Controls
//io.ConfigFlags |= ImGuiConfigFlags_NavEnableGamepad; // Enable Gamepad Controls
io.ConfigFlags |= ImGuiConfigFlags_DockingEnable; // Enable Docking
io.ConfigFlags |= ImGuiConfigFlags_ViewportsEnable; // Enable Multi-Viewport / Platform Windows
Expand Down
154 changes: 154 additions & 0 deletions timeline.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,23 @@ void DrawItem(
ImGui::BeginGroup();

ImGui::InvisibleButton("##Item", size);

// Don't skip invisible item if it is the item we have just selected
if (!ImGui::IsItemVisible()
&& appState.selected_object == item
&& (appState.scroll_left || appState.scroll_right)) {

otio::Item* selected_item = dynamic_cast<otio::Item*>(appState.selected_object);
if (appState.scroll_right) {
ImGui::SetScrollX( ImGui::GetScrollX() +
selected_item->range_in_parent().duration().to_seconds() * scale);
appState.scroll_right = false;
}
if (appState.scroll_left) {
ImGui::SetScrollX(selected_item->range_in_parent().start_time().to_seconds() * scale);
appState.scroll_left = false;
}
}
if (!ImGui::IsItemVisible()) {
// exit early if this item is off-screen
ImGui::EndGroup();
Expand Down Expand Up @@ -290,6 +307,24 @@ void DrawTransition(

ImVec2 p0 = ImGui::GetItemRectMin();
ImVec2 p1 = ImGui::GetItemRectMax();

// Don't skip invisible item if it is the item we have just selected
if (!ImGui::IsItemVisible()
&& appState.selected_object == transition
&& (appState.scroll_left || appState.scroll_right)) {

otio::Transition* selected_item = dynamic_cast<otio::Transition*>(appState.selected_object);
if (appState.scroll_right) {
ImGui::SetScrollX( ImGui::GetScrollX() +
selected_item->range_in_parent()->duration().to_seconds() * scale);
appState.scroll_right = false;
}
if (appState.scroll_left) {
ImGui::SetScrollX( selected_item->range_in_parent()->start_time().to_seconds() * scale);
appState.scroll_left = false;
}
}

if (!ImGui::IsRectVisible(p0, p1)) {
ImGui::EndGroup();
ImGui::PopID();
Expand Down Expand Up @@ -1490,6 +1525,125 @@ void DrawTimeline(otio::Timeline* timeline) {
appState.scroll_to_playhead = false;
}

if (ImGui::IsWindowFocused()){
// Right arrow
if (ImGui::IsKeyPressed(ImGuiKey::ImGuiKey_RightArrow)) {
std::string selected_type = appState.selected_object->schema_name();
if (selected_type == "Clip" || selected_type == "Gap" || selected_type == "Transition") {
// Loop through selected items parent track to find the next item
auto parent = dynamic_cast<otio::Composable*>(appState.selected_object)->parent();
for(auto it = parent->children().begin(); it != parent->children().end(); it++ ){
// If last item then do nothing
if (std::next(it) == parent->children().end()) {
break;
}
if (*it == appState.selected_object) {
std::advance(it, 1);
SelectObject(*it);
appState.scroll_right = true;
break;
}
}
}
}
// Left Arrow
if (ImGui::IsKeyPressed(ImGuiKey::ImGuiKey_LeftArrow)) {
std::string selected_type = appState.selected_object->schema_name();
if (selected_type == "Clip" || selected_type == "Gap" || selected_type == "Transition") {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of checking the schema_name, it would be safer and shorter to check if the dynamic_cast to Composable returned a value. If it was non-null, then you can find the parent. For safety you might also want to check that parent() was non-null, since the top-level stack won't have a parent. That will cover all the generalized cases - though the left/right navigation is really assuming the parent is a Track (vs a Stack).

So maybe the logic could be:

  • Try dynamic_cast to Composable.
  • Get the parent()
  • Check that the parent is not null
  • Check that the parent is a Track

// Loop through selected items parent track to find the previous item
auto parent = dynamic_cast<otio::Composable*>(appState.selected_object)->parent();
for(auto it = parent->children().begin(); it != parent->children().end(); it++ ){
if (*it == appState.selected_object) {
// If first item do nothing
if (it == parent->children().begin()) {
break;
}
std::advance(it, -1);
SelectObject(*it);
appState.scroll_left = true;
break;
}
}
}
}

// The Stacks of video and audio Tracks go in opposite directions
// therefore the logic for the for the Up Arrow on Video tracks is
// the same as the logic for Down Arrow on Audio tracks and vice versa

auto parent = dynamic_cast<otio::Composable*>(appState.selected_object)->parent();
auto selected_track = dynamic_cast<otio::Track*>(parent);
std::string selected_track_type = selected_track->kind();

if ((ImGui::IsKeyPressed(ImGuiKey::ImGuiKey_DownArrow)) ||
(ImGui::IsKeyPressed(ImGuiKey::ImGuiKey_UpArrow))) {

// Only run if the right type is selected
std::string selected_type = appState.selected_object->schema_name();
if (selected_type == "Clip" || selected_type == "Gap" || selected_type == "Transition") {

// Finding start time varies depending on object type
otio::RationalTime start_time;
if (selected_type == "Clip" || selected_type == "Gap") {
auto child = dynamic_cast<otio::Item*>(appState.selected_object);
start_time = child->range_in_parent().start_time();
} else if (selected_type == "Transition") {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could avoid this special case by using parent->range_of_child(child) which works on all Composables.

auto child = dynamic_cast<otio::Transition*>(appState.selected_object);
start_time = child->range_in_parent().value().start_time();
}

// Get the parent object and track stack
auto parent = dynamic_cast<otio::Composable*>(appState.selected_object)->parent();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's already a parent variable on line 1574.

auto tracks = dynamic_cast<otio::Stack*>(parent->parent());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You'll need to check that tracks was non-null. If you have an OTIO with nested compositions, the parent's parent might not be a Stack. At the moment, I'm not sure you could end up selecting such an item in Raven, but we will allow for it eventually.


// Loop through tracks until we find the current one
for(auto it = tracks->children().begin(); it != tracks->children().end(); it++ ){
otio::Composable* track = *it;
if (track == parent) {
// Down Arrow and Video or Up Arrow and Audio
if ((ImGui::IsKeyPressed(ImGuiKey::ImGuiKey_DownArrow) && selected_track_type == "Video") ||
(ImGui::IsKeyPressed(ImGuiKey::ImGuiKey_UpArrow) && selected_track_type == "Audio")) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oof, sorry about this track order hassle. I often wish we had designed Timeline to have separate lists for video and audio (or other) tracks. I suppose Raven could make separate lists when it first loads? Anyways, the solution you have here is fine.

// If first item then do nothing
if (it == tracks->children().begin()) {
break;
}
// Select the next track up
std::advance(it, -1);

// Up Arrow and Video or Down Arrow and Audio
} else if ((ImGui::IsKeyPressed(ImGuiKey::ImGuiKey_UpArrow) && selected_track_type == "Video") ||
(ImGui::IsKeyPressed(ImGuiKey::ImGuiKey_DownArrow) && selected_track_type == "Audio")) {
// If last item then do nothing
if (std::next(it) == tracks->children().end()) {
break;
}
// Select the next track up
std::advance(it, 1);
} else{
break;
}

otio::Composable* next_it = *it;
otio::Track* next_track = dynamic_cast<otio::Track*>(next_it);

// Only iterate over tracks of the same kind
if(next_track->kind() != selected_track_type){
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you need to check for next_track being null here?

break;
}

// If there is an iten that overlaps with the current selection's start time
// select it
if (next_track->child_at_time(start_time)){
SelectObject(next_track->child_at_time(start_time));
break;
}
}
}
}
}

}

// This is helpful when debugging visibility performance optimization
// ImGui::SetTooltip("Tracks rendered: %d\nItems rendered: %d",
// __tracks_rendered, __items_rendered);
Expand Down
Loading