[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 ? |
damienmarchal
changed the title
damienmarchal
changed the title
damienmarchal
changed the title
damienmarchal
changed the title
hugtalbot
changed the title
…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: