Move FPS timer to top-right, next to console badge#2822
Move FPS timer to top-right, next to console badge#2822dhimasardinata wants to merge 8 commits intof3d-app:masterfrom
Conversation
5b80333 to
68962f9
Compare
|
Hi @dhimasardinata , I think this can be integrated easily, are you around If I review soon ? |
|
@mwestphal sure, I'm around 👍 |
|
Please comment on #2803 so I can assign it to you |
There was a problem hiding this comment.
please comment on the issue.
|
Nice! You need to update test baselines to account for the move, do you know how to do that ? |
|
Thanks for the heads up! I've updated the TestFPS baseline and verified that all tests are passing now. Let me know if you spot anything else! |
Im afraid the test is not right because it would result in a different image each time. A solution is to use Let me know if you need help. |
|
I updated the FPS offset so it only shifts when the badge is actually visible, and I used the badge size for spacing. I also added --font-scale=0.5 to TestFPS, refreshed the baseline, and re-ran it locally. |
Nice! Let see how CI goes. |
|
\ci fast |
|
\ci full |
| void ShowBadge(); | ||
|
|
||
| /** | ||
| * Check if the console badge is visible |
There was a problem hiding this comment.
| * Check if the console badge is visible | |
| * Return true if the console badge is visible, false otherwise |
|
@dhimasardinata let me know if you need help making the test work, dont hesitate to join the discord to discuss: https://discord.f3d.app |
|
@mwestphal hey, yes i need help getting the test to work😅 thanks! |
|
Just increase the threshold of the test ? |
|
Still working on this @dhimasardinata ? |
Only offset the FPS counter when the console badge is visible and use the badge size for spacing. Also reduce font scale for TestFPS and update baseline.
Put console badge lookup on a single line to match formatting.
Use Crosterian test font with a smaller scale for TestFPS and update the baseline.
a0d1e2e to
bd01d5c
Compare
|
Hey @mwestphal, sorry for the delay! I'm still working on this. I've rebased on master and increased the TestFPS threshold to 0.2 as you suggested. The FPS counter displays dynamic values that vary across CI machines, so the higher threshold should accommodate that. Let me know if there's anything else needed! |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #2822 +/- ##
=======================================
Coverage 97.14% 97.15%
=======================================
Files 209 209
Lines 16554 16562 +8
=======================================
+ Hits 16082 16090 +8
Misses 472 472 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Thanks! Next time please request a review (top right) when you need it :) |
…or TestFPSWithBadge
|
@mwestphal I've addressed your review comments: |
please resolve adressed command and request a review. Top right. |
| # Require improved importer support https://gitlab.kitware.com/vtk/vtk/-/merge_requests/11303 | ||
| if(VTK_VERSION VERSION_GREATER_EQUAL 9.3.20240910) | ||
| f3d_test(NAME TestFPSWithBadge DATA invalid_body.vtp ARGS -z --font-scale=0.35 | ||
| --font-file=${F3D_SOURCE_DIR}/testing/data/Crosterian.ttf NO_DATA_FORCE_RENDER UI NO_BASELINE) |
There was a problem hiding this comment.
why NO_RENDER NO_BASELINE ? testing visually the UI placement would be nice imo.
Move the FPS timer from the bottom-left (where it overlaps with the Axis widget X) to the top-right corner, next to the console badge.
When the console badge is visible, the FPS timer is positioned to its left with standard padding. When the console badge is not visible, it is positioned at the screen edge.
Fixes #2803