Skip to content

YAPSP: TableView/QAbstractTableModel based parameter list#12876

Merged
DonLakeFlyer merged 1 commit intomasterfrom
YAPSP
May 24, 2025
Merged

YAPSP: TableView/QAbstractTableModel based parameter list#12876
DonLakeFlyer merged 1 commit intomasterfrom
YAPSP

Conversation

@DonLakeFlyer
Copy link
Collaborator

@DonLakeFlyer DonLakeFlyer commented May 19, 2025

Yes Another Parameter Search Pull. TableView/QAbstractItemModel basically achieves the best of both worlds with respect to ListView caching and only created visible items as well as more control over column visuals. This is fully implemented now.

Replacement for: #12869 and #12866

@DonLakeFlyer DonLakeFlyer marked this pull request as draft May 19, 2025 01:36
@DonLakeFlyer
Copy link
Collaborator Author

@rubenp02, @HTRamsey FYI so you guys can try the perf.

Also my take on mobile is that it should not be typedown style search but have a real "Search" button to trigger a search.

This was changed due to performance problems with the GridLayout based approach. TableView give us windowing over the rows so we don't instantiate them until needed.
@DonLakeFlyer DonLakeFlyer marked this pull request as ready for review May 19, 2025 21:33
@DonLakeFlyer DonLakeFlyer requested a review from HTRamsey May 19, 2025 21:33
@DonLakeFlyer DonLakeFlyer merged commit 22641bc into master May 24, 2025
16 checks passed
@DonLakeFlyer DonLakeFlyer deleted the YAPSP branch May 24, 2025 01:55
@rubenp02
Copy link
Contributor

I'm very late, but I finally had the time to test this, and it seems perfect. Thank you so much! Amazing job.

I recorded a performance/feel comparison with one of our SkyDroid H16 because I think the performance was a lot worse than with your Herelink or Galaxy Tab:

param-list-perf-comparison.mp4

I was pretty sure the main issue was the regexp JIT stuff, so I included a build with just that fix in the comparison. But that clearly wasn't it, since it's only a tiny bit faster, if at all. The fixes in this PR make it work perfectly again (and IMO an explicit search button isn't needed with this kind of performance). I forgot to include a clip of stable as a baseline but for reference, it feels as good as with this fix.

@DonLakeFlyer
Copy link
Collaborator Author

DonLakeFlyer commented May 26, 2025

@rubenp02 Thanks so much for verifying. Bit of a wild ride in determining the real problem here :)

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.

3 participants