Repository navigation
Conversation
Qt's cache buffer makes them later, from a 16 ms timer of its own, and a race in Qt can leave that timer running: an idle window then draws a frame every refresh. The list and the columns make the same rows at once with display margins; the grid keeps its own row past each edge.
[Medium impact] The PR appears safe to merge; no actionable defects were found. SummaryTurns off Qt's deferred cache buffer in the folder views to avoid the reported idle redraw race.
Reviews (1) · Last reviewed commit: "Make the rows past a view's edges with t..." · Reviewed by Greptile |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
One fix, in one commit with its own test: now and then a Folio window that is doing nothing keeps drawing a frame at every refresh of the screen, with its render thread on about half a core, until it is closed. A race in Qt sets it off, and the cache buffer of Folio's views is what walks into it. The views now make their rows without that buffer. The suite goes from 194 to 197 tests. No library,
dependsoroptdependsentry is added.What you'd notice
Numbers
How often it happens depends on timing, so the counts are small and uneven. Measured on a mini PC, cold starts on a folder of 2,000 videos and pictures:
main, this weekIn each of the four caught on 0.1.1 a probe counted 479 or 480 frames in every 4 seconds on a still, complete window (a 120 Hz screen), for the two minutes it was watched. The one this week used 42.6 s of CPU in the measured window, against about 11 s for its neighbours.
The same folder, thumbnails made anew at each start, page cache dropped, 6 runs a view,
mainand this branch in one session (the build measured has this commit's view settings; review since then named the 320 and tightened the test):mainmainmainmainOn a folder of 100,000 files the two are level in every view: first window 894 to 971 ms against 904 to 959 ms in the grid, memory 235.6 to 236.0 MiB against 229.5 to 240.3 MiB, and the same for the list and the columns.
Holding Down through 300 files in the grid, measured on 0.1.1 with the same change: the time from the key to its frame was 10.3 ms at the median either way, and 13.9 ms at most against 14.9 ms. No frame took longer than one refresh at 60 Hz.
How it works
ListVieworGridViewwith a cache buffer (Qt's default is 320 pixels) fills that buffer later than the rows it shows: each refill starts a 16 ms pause job,bufferPause, inQQuickItemViewPrivate::refill().QQmlAnimationTimer, a queuedstopTimeris skipped while a start is pending. If the pending job is then stopped while it is still inanimationsToStart, it is taken off that list and nothing queues another stop. The animation timer stays registered with no job, the animation driver never stops, and the window is redrawn at every refresh. This is Qt Declarative 6.12.0, and the same code is in Qt's dev branch.cacheBuffer: 0no pause job is ever started, so the views cannot reach that state.displayMarginBeginninganddisplayMarginEndof 320 pixels, whichrefill()fills at once, with the rows shown and with no timer: the rows that exist are the ones that existed before.GridViewmakes a row past each edge by itself, and rows inside a display margin are drawn, not only made: every tile is a layer, and margins there cost 7 MiB on the large folder.Tests
One new case with a row for each view (grid, list, columns), failing for all three without the commit. It opens the real pane on a folder of 60 entries in a short window, moves the view to its end and back, and checks that in the list and the columns rows reach 320 pixels behind the top edge and as far past the bottom one at once, and that in every view no row is added in the 300 ms after: a cache buffer's timer adds its rows in that time. Eleven mutants, each taking one of the new properties from one view or halving the 320, all fail it.
Limits
mainand rows would be made as they scroll in; I kept the rows they had.maindid not show it in this week's 54 starts.