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.

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

Render annotation flags over track group headers #97

Open
wants to merge 3 commits into
base: reuse_timeline
Choose a base branch
from

Conversation

cdamus
Copy link
Collaborator

@cdamus cdamus commented Feb 28, 2024

All track rendering was done on a big scrolling canvas element that sits behind the track dividing lines and shells. That is fine for rendering stuff in the track content area of the scrolling tracks, but the track content area of the headers of expanded groups does not scroll with the rest. They're not even a part of the same static layout, instead being positioned relatively and moreover sticking to the top of the panel as tracks scroll up beneath them. Because they are in a different layout to the canvas, they occlude anything painted on the canvas, so painting in the expanded group headers has no effect.

Consequently, it is necessary to give each expanded group header its own canvas on which we can draw content such as the stems of note flags.

CleanShot 2024-02-28 at 12 57 14

All track rendering was done on a big scrolling canvas element that sits
behind the track dividing lines and shells. That is fine for rendering
stuff in the track content area of the scrolling tracks, but the track
content area of the headers of expanded groups does not scroll with the
rest. They're not even a part of the same static layout, instead being
positioned relatively and moreover sticking to the top of the panel as
tracks scroll up beneath them. Because they are in a different layout
to the canvas, they occlude anything painted on the canvas, so painting
in the expanded group headers has no effect.

Consequently, it is necessary to give each expanded group header its own
canvas on which we can draw content such as the stems of note flags.

Signed-off-by: Christian W. Damus <[email protected]>
@ALevansSamsung
Copy link
Collaborator

ALevansSamsung commented Feb 28, 2024

Adding or removing a note while the sticky open track group is still there tends to make the annotations drawn on it stale.

staleNoteLines-2024-02-28_12.47.34.mp4

The algorithm for determining what needs to be redrawn didn't account
for the fact that a group header that would be scrolled offscreen and
so not need redrawing when collapsed might be sticky and still onscreen
when expanded. So now we redraw an expanded group header if it really
seems to be onscreen _or_ if any member within it (recursively) is
onscreen (which would imply that the group header is sticky at the top.)

Signed-off-by: Christian W. Damus <[email protected]>
@cdamus
Copy link
Collaborator Author

cdamus commented Feb 28, 2024

Adding or removing a note while the sticky open track group is still there tends to make the annotations drawn on it stale.

Great find! Thanks. That was a tricky case and should be fixed in commit 4a6af55.

@ALevansSamsung
Copy link
Collaborator

While the annotation lines are no longer stale, the ones added while the sticky header is up are not the same length
image

@cdamus
Copy link
Collaborator Author

cdamus commented Feb 29, 2024

While the annotation lines are no longer stale, the ones added while the sticky header is up are not the same length

Ah. What's happening here is that a group header has tucked up underneath another header that is not as high. In light mode, you would see that the header on top has a line ① running along its bottom edge and the apparent gap ② is just where the header underneath doesn't know it still needs to be drawn.

CleanShot 2024-02-29 at 08 18 10

@cdamus
Copy link
Collaborator Author

cdamus commented Feb 29, 2024

While the annotation lines are no longer stale, the ones added while the sticky header is up are not the same length

@ALevansSamsung this should be fixed with commit b450b90 which, overall, simplifies the code greatly by not trying to be clever about efficient drawing of something is known not to be complex to draw 😀

@ALevansSamsung
Copy link
Collaborator

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants