OpenTimelineIO
OpenTimelineIO copied to clipboard
AcademySoftwareFoundation
Fix svg target url bug
Potentially starts to fix the below issues.
Looking for feedback on this approach with the svg adapter and then I'll move on to the other adapters
I think this looks reasonable. Have you got a before/after svg rendering to show what the impact of the changes is?
The SVG adapter was aiming to create diagrams similar to the ones on this documentation page: https://opentimelineio.readthedocs.io/en/latest/tutorials/otio-timeline-structure.html
Those were made by hand (and I'd love to not have to do it by hand again...) One of the tricky things that came up when we made those was trying to find a clear way to show when the source_range was or was not set. If you look closely, some of them intentionally have no source_range in an attempt to show how available_range is used instead. However that important detail is really hard to see, even if you're looking for it.
For this PR, you have sort of the opposite problem. If available_range is not know, then you can guess that the source_range could be used instead - but it hides an important detail - that available_range is null.
I wonder if there's a way to draw attention to the fact that either available_range or source_range is missing? Note: If both are missing, then the clip is invalid and has no way of knowing its duration.
Thanks for the reminder @reinecke! I completely forgot about this PR. I think the actual fixes this PR introduced have been lost because of all the code style choices I made and in hindsight shouldn't have done. I will go through git history and pull over the necessary stuff to the svg adapter repo as you described and then close out this PR.
Closing this PR because it was continued in the SVG adapter repo: https://github.com/OpenTimelineIO/otio-svg-adapter/pull/7 and is out of date with the current state of main in the OTIO repo.