Skip to content

[Fix] Do not set axes grid center position #2824#2947

Merged
mwestphal merged 6 commits intof3d-app:masterfrom
cuongdv-20:fix_axes_grid_position
Mar 21, 2026
Merged

[Fix] Do not set axes grid center position #2824#2947
mwestphal merged 6 commits intof3d-app:masterfrom
cuongdv-20:fix_axes_grid_position

Conversation

@cuongdv-20
Copy link
Copy Markdown
Contributor

@cuongdv-20 cuongdv-20 commented Mar 15, 2026

Describe your changes

Function ConfigureGridAxesUsingCurrentActors() sets the grid axes position to center of bounding box

double center[4] = { 0, 0, 0, 1 };
bbox.GetCenter(center);
this->GridAxesActor->SetPosition(center);

This will shift the axes grid toward center of the object, but apply grid bound from object's bounding box.
The problem occurs to all objects that are deviated to one side of the axis

Issue ticket number and link if any

#2824

Checklist for finalizing the PR

  • I have performed a self-review of my code
  • I have added tests for new features and bugfixes
  • I have added documentation for new features
  • If it is a modifying the libf3d API, I have updated bindings
  • If it is a modifying the .github/workflows/versions.json, I have updated docker_timestamp

Continuous integration

Please write a comment to run CI, eg: \ci fast.
See here for more info.

@mwestphal mwestphal self-requested a review March 15, 2026 08:39
Comment thread vtkext/private/module/vtkF3DRenderer.cxx
Comment thread testing/recordings/TestInteractionAxesGridPosition.log Outdated
Comment thread application/testing/tests.interaction.cmake Outdated
Copy link
Copy Markdown
Member

@mwestphal mwestphal left a comment

Choose a reason for hiding this comment

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

Some questions, please, whenever you need, ask for a review using github feature top right.

@cuongdv-20 cuongdv-20 requested a review from mwestphal March 15, 2026 11:54
Copy link
Copy Markdown
Member

@mwestphal mwestphal left a comment

Choose a reason for hiding this comment

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

some comments

@cuongdv-20 cuongdv-20 requested a review from mwestphal March 15, 2026 14:06
@mwestphal
Copy link
Copy Markdown
Member

\ci fast

Copy link
Copy Markdown
Member

@mwestphal mwestphal left a comment

Choose a reason for hiding this comment

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

a question

Comment thread application/testing/tests.interaction.cmake Outdated
@cuongdv-20 cuongdv-20 requested a review from mwestphal March 16, 2026 03:32
@mwestphal
Copy link
Copy Markdown
Member

\ci full

@mwestphal
Copy link
Copy Markdown
Member

please rebase on latest master @cuongdv-20

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Mar 17, 2026

You are modifying versions.json, please update the docker timestamp as well, this will generate new docker images and caches needed for CI.

@mwestphal
Copy link
Copy Markdown
Member

rebase is incorrect, please read up on rebase: https://thepugautomatic.com/2017/02/interactive-rebase-against-the-remote-master/

@codecov
Copy link
Copy Markdown

codecov Bot commented Mar 17, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.13%. Comparing base (3a6f484) to head (8026282).
⚠️ Report is 5 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #2947   +/-   ##
=======================================
  Coverage   97.12%   97.13%           
=======================================
  Files         206      206           
  Lines       16411    16455   +44     
=======================================
+ Hits        15939    15983   +44     
  Misses        472      472           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@cuongdv-20 cuongdv-20 force-pushed the fix_axes_grid_position branch from 47b7289 to 8026282 Compare March 19, 2026 16:22
@mwestphal
Copy link
Copy Markdown
Member

\ci full

Copy link
Copy Markdown
Member

@mwestphal mwestphal left a comment

Choose a reason for hiding this comment

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

LGTM, lets see what CI says.

@mwestphal mwestphal self-requested a review March 20, 2026 11:09
@mwestphal
Copy link
Copy Markdown
Member

CI is clean, lets merge

@mwestphal mwestphal merged commit 52eabba into f3d-app:master Mar 21, 2026
88 checks passed
@mwestphal
Copy link
Copy Markdown
Member

Thanks for your contribution @cuongdv-20 !

PedroAbreuMaia pushed a commit to PedroAbreuMaia/f3d that referenced this pull request Mar 26, 2026
* do not set axes grid center position

* update test interaction

* Not use interaction test

* do not set axes grid center position

* update test interaction

* Not use interaction test
PedroAbreuMaia pushed a commit to PedroAbreuMaia/f3d that referenced this pull request Mar 26, 2026
* do not set axes grid center position

* update test interaction

* Not use interaction test

* do not set axes grid center position

* update test interaction

* Not use interaction test
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants