-
Notifications
You must be signed in to change notification settings - Fork 311
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[SofaKernel] Add defaulttype::RGBAColor #119
[SofaKernel] Add defaulttype::RGBAColor #119
Conversation
I have added a RGBAColor defaulttype There is several advantage of doing so: - no ambiguity in the code between a rgba color and vector4 - factor the code needed to build colors from their name, hexadecimal or list of float - possibility in the different sofa user interfaces to use color picker widget instead of vector one for selecting the colors. There is the corresponding test. I'm following a bottom-up approach, consequently the implementation is on purpose not templated until it is proven that we really need other color coding.
It is possible to write stuff like that: RGBAColor::white() == RGBAColor(1,1,1,1);
To any win32 user...any idea where the problem can come from ? |
…colors. It is now possible to pass "1 1 1" string to a RGBAColor to build the rgba color: "1 1 1 1"
The compilation problem with visual 2015 seems related to template instantiation and the way the compiler handles static_assert in fixed_array. I have no idea how to fix that so I passed the object into a template version. If this solve the issue this will be definitely the most un-satisfactory commit of my all life uglyfing a very readable and clear implementation to turn it into a messy useless templates. I secretely hope this commit will not fix the problem :)
In this commit I have added the use of the std::enable_if to insure that the non needed versions of the function are not implicitely generated. Wait & see if this work.
I forgot that I remove two include... which of course prevent sofa to compile.
Would it be possible to get such color api in the python binding VisualModel::setColor? ^^ |
@matthieu-nesme, thanks for your interest, In this PR set up the base of the API, VisualModel is not in PR #124 because the color is not a Data field of the component. So I let it away for the moment. But you are right this should be done and more generally it would be nice to have the RGBAColor object be exposed as a python object. |
This reverts commit 74b9f31.
I checked from my windows build and apparently : none of the functions templated with the enable_if trick "int NN = N, typename std::enable_if<(NN==X), etc..." are built and exported into the dll. Errors are spotting a lack of symbols from TRGBAColor r(); g(), b(), a() Sorry if I'm stating things you already know |
Ready to be merged. |
Just to be "tatillon", are the modifications with the "std::enable_if" really related to this PR ? |
@thomas-lemaire please be tatillon because it is a good question :) Initially it was not part of the PR (have a look at the first commit to see what was my initial code) but on windows the static_asserts were causing compilation failure on V2015 and VS2013 builds. After some digging and request for discussions (in issue: #130)) my conclusion was that the failure was not because of my code but because that somehow there was implicit instanciation of the function like Vec<4,float>::set(...,...,N...) with N!=4. And as they were instanciated this was causing the static_assert to reject the code when N==4. I have the feeling that the static_assert in Vec was only working because of side effects on way compiler interleave optimization and instantiation of templates (but I may be totally wrong). Basically all the set(...,) function are present but they were removed because they were not used before the check by static_assert. So I try to implement a corrected version of the expected behavior by using the enable_if feature of std::x11 which is generating the right function but not the others. |
Ok thanks ! |
This PR adds a RGBAColor type in defaulttype as well as dedicated tests.
There is several advantage of doing so:
- no ambiguity in the code between a rgba color and vector4.
- the code needed to build colors from their name, hexadecimal or list of float is factored in this class (while currently there is at least 8 duplications in the sofa code base)
- unifying the underlying data also offer the possibility to offer consistent user interface for colors
This PR will be the ground to solve issue ##64.
This PR have no impact on the sofa source code.
Checklist to be merge: