FreeCAD
[Gui] Axis colors changeable
@benj5378 @PaddleStroke maybe this can already be considered in / for https://github.com/FreeCAD/FreeCAD/pull/16675
https://github.com/FreeCAD/FreeCAD/pull/16675 is already a huge PR so perhaps it's best not to overload it. But yes ultimately the new core LCS will need to be customizable same as the others.
Yes or the other way around. If this merge first, then @benj5378 can advise me how to set the colors correctly in my pr.
#16675 is already a huge PR so perhaps it's best not to overload it. But yes ultimately the new core LCS will need to be customizable same as the others.
Using this colors is literally calling Gui::ViewParams::instance()->getAxisXColor() to obtain the color. It won't affect your PR much, this method is already available and the PR should use it before it gets merged. No reason to postopone changes like this for later as the later can never come and we would end up with yet another inconsistency.
@benj5378 as this change affects settings UI can you add screenshot of before and after?
Ah ok then, I'll check it
On Sat, Oct 5, 2024, 10:46 Kacper Donat @.***> wrote:
#16675 https://github.com/FreeCAD/FreeCAD/pull/16675 is already a huge PR so perhaps it's best not to overload it. But yes ultimately the new core LCS will need to be customizable same as the others.
Using this colors is literally calling Gui::ViewParams::instance()->getAxisXColor() to obtain the color. It won't affect your PR much, this method is already available and the PR should use it before it gets merged. No reason to postopone changes like this for later as the later can never come and we would end up with yet another inconsistency.
— Reply to this email directly, view it on GitHub https://github.com/FreeCAD/FreeCAD/pull/16995#issuecomment-2394984272, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEYO6MJYUI5CEWTT5GBUKBTZZ6RNTAVCNFSM6AAAAABPKWVWL2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGOJUHE4DIMRXGI . You are receiving this because you were mentioned.Message ID: @.***>
Using this colors is literally calling Gui::ViewParams::instance()->getAxisXColor() to obtain the color.
So my PR was already using this actually. So it's already compliant then
@benj5378 which axes colors does this affect?Transform dragger, origin datums, axis cross, the lines on the navi cube and the little one in the corner?
@benj5378 which axes colors does this affect?
That's uncertain, and depends on the implementation of getAxisXColor() across the codebase. @maxwxyz
This PR adds UI for setting the axis colors property (returned by getAxisXColor()). It further implements the axis shown beneath to use getAxisXColor().
If there are axis not responding to the preference change, it's because the axis colors have been hardcoded. Depending on implementation in the specific code locations, FreeCAD might need restart for changed axis colors to take effect. @PaddleStroke added functionality, such that datum planes are also using getAxisXColor() and thus affected by this PR.
@kadet1090, good points that I agree with. Let that be another PR.
I guess we are ready for merge then.
@chennes I have to say there seems to be some double standards going on recently, I was specifically told that new code wasn't to have C strcmp and that's exactly what has been merged here, if there's rules, fine but they need to be applied to everyone IMHO.
@Syres916 much as I hate strcmp and really, really hate to see it in new code, in this particular case the new code is part of a chain of if statements that all use that structure. So rather than forcing this PR to do a big refactoring, we decided to let the strcmp in to maintain consistency in a very local code block. I'd still very much welcome a refactoring-only PR that eliminates the old-school C style calls.