Skip to content

Fix: panel resizing created jankiness with dynamically sized widgets in the panel - #175

Open
Bilalharoon wants to merge 5 commits into
luisbocanegra:mainfrom
Bilalharoon:main
Open

Fix: panel resizing created jankiness with dynamically sized widgets in the panel#175
Bilalharoon wants to merge 5 commits into
luisbocanegra:mainfrom
Bilalharoon:main

Conversation

@Bilalharoon

Copy link
Copy Markdown

Follow-up to #173 fixing two issues that surfaced when the panel is resized and the bar count needs to be recalculated:

  1. Flicker on restart: Resizing triggers a Cava restart, which briefly disabled the widget. This caused a visible flicker and let the panel recalculate sizing incorrectly mid-transition. Added a restarting flag so the widget doesn't disable itself during these intentional, resize-triggered restarts.

  2. Zero-length bar array during restart: While Cava restarts, barCount briefly reports 0, which fed bad values into the canvas repaint. The canvas now only repaints when the value array length is greater than 1, so instead of flickering off and on, the visualization freezes on its last frame and resumes once Cava is back up.

@luisbocanegra luisbocanegra left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Thank you for working on this, it's way better now!

Some observations and nitpicks:

  • I like to keep the code formatted with qmlformat, I use the Qt Qml vscode extension for this https://marketplace.visualstudio.com/items?itemName=TheQtCompany.qt-qml but it can be done manually from the cli

  • Try to commit related changes together, and only keep meaningful commits, for example (from git log --oneline):

    2ae69cc fix: barcount only updates when necessary
    c44eaef fix: removed an unused property
    45053e2 fix: removed an unnecessary logging artifact
    3c8007c fix: bars no longer dissapear during resize
    23c3cc4 fix: Panel widget disappearing during panel resize
    

    Could be collapsed into:

    2ae69cc fix: barcount only updates when necessary
    3c8007c fix: visualizer disappears during resize
    23c3cc4 fix: widget disappearing during panel resize
    

    There are multiple ways to do this https://stackoverflow.com/questions/5189560/how-do-i-squash-my-last-n-commits-together, an interactive rebase, restoring the last 5 commits and committing new commits manually, checking out to another branch and cherry-picking

    If you don't feel very confident running these commands you can always git checkout to another branch and experiment there

  • It looks like the email you configured for git is not associated with your GitHub account, if you like yo have your account be listed in contributors you will have to update it and amend the author of your commits https://docs.github.com/en/account-and-profile/how-tos/email-preferences/setting-your-commit-email-address

    Then you can do an interactive rebase of the latest N commits to to update the author with the updated one:

    git rebase -i HEAD~N
    

    Then this in each step of the rebase:

    git commit --amend --reset-author --no-edit
    git rebase --continue
    

If you want, I can do all the first points and you only do the last 🙂

Also last but not least, always create pull requests from a branch that is not main, in your previous pull request I accidentally reset your branch, because I cloned into local branch pr-173 but accidentally force pushed to your remote main using git push -f Bilalharoon main 😅, though this made me realize that it can do git push Bilalharoon HEAD:main so it should be fine if we continue in this pr, but for future pull requests (either here or other repositories) a non-main branch is better and often mandatory depending on the repository owners.

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