One of the worst routines I've ever written. Shame on me?
-
Forgive me for what I'm about to paste. The whole grotty mess is in the
convert<>()function at the bottom of this horrible file: gfx/include/gfx_pixel.hpp[^] I've omitted several pages of code. The worst/best part of this is it's all done at compile time, and generates at most several lines of asm, but often a lot less, and in cases where the input values are known at compile time the entire computation is done at compile time and generates exactly no binary code. Despite that, it is as if it was written at gunpoint. I could break it up into a ton of nasty routines but that just feels like spreading the mess around. I'm ready for the flaming. :~// converts a pixel to the destination pixel type
template
constexpr static inline gfx_result convert(PixelTypeLhs source,PixelTypeRhs* result,const PixelTypeRhs* background=nullptr) {
static_assert(helpers::is_same::value || !PixelTypeLhs::template has_channel_names::value,"left hand pixel must not be indexed");
static_assert(helpers::is_same::value || !PixelTypeRhs::template has_channel_names::value,"right hand pixel must not be indexed");
if(nullptr==result) return gfx_result::invalid_argument;
if(helpers::is_same::value) {
if(nullptr==background) {
result->native_value=source.native_value;
return gfx_result::success;
} else {
return gfx_result::invalid_format;
}
}
bool good = false;
PixelTypeRhs tmp;
typename PixelTypeRhs::int_type native_value = tmp.native_value;// here's where we gather color model information using is\_rgb = typename PixelTypeLhs::template has\_channel\_names; using is\_yuv = typename PixelTypeLhs::template has\_channel\_names; using is\_ycbcr = typename PixelTypeLhs::template has\_channel\_names; using is\_hsv = typename PixelTypeLhs::template has\_channel\_names; using is\_hsl = typename Pixe -
Forgive me for what I'm about to paste. The whole grotty mess is in the
convert<>()function at the bottom of this horrible file: gfx/include/gfx_pixel.hpp[^] I've omitted several pages of code. The worst/best part of this is it's all done at compile time, and generates at most several lines of asm, but often a lot less, and in cases where the input values are known at compile time the entire computation is done at compile time and generates exactly no binary code. Despite that, it is as if it was written at gunpoint. I could break it up into a ton of nasty routines but that just feels like spreading the mess around. I'm ready for the flaming. :~// converts a pixel to the destination pixel type
template
constexpr static inline gfx_result convert(PixelTypeLhs source,PixelTypeRhs* result,const PixelTypeRhs* background=nullptr) {
static_assert(helpers::is_same::value || !PixelTypeLhs::template has_channel_names::value,"left hand pixel must not be indexed");
static_assert(helpers::is_same::value || !PixelTypeRhs::template has_channel_names::value,"right hand pixel must not be indexed");
if(nullptr==result) return gfx_result::invalid_argument;
if(helpers::is_same::value) {
if(nullptr==background) {
result->native_value=source.native_value;
return gfx_result::success;
} else {
return gfx_result::invalid_format;
}
}
bool good = false;
PixelTypeRhs tmp;
typename PixelTypeRhs::int_type native_value = tmp.native_value;// here's where we gather color model information using is\_rgb = typename PixelTypeLhs::template has\_channel\_names; using is\_yuv = typename PixelTypeLhs::template has\_channel\_names; using is\_ycbcr = typename PixelTypeLhs::template has\_channel\_names; using is\_hsv = typename PixelTypeLhs::template has\_channel\_names; using is\_hsl = typename PixeMind you, I'm not going to talk about the lack of comments. I also freely admit that templates make me uncomfortable, but how in the hell would you debug/test that? If all of that generates a few lines of assembly... :confused:
Charlie Gilley “They who can give up essential liberty to obtain a little temporary safety deserve neither liberty nor safety.” BF, 1759 Has never been more appropriate.
-
Mind you, I'm not going to talk about the lack of comments. I also freely admit that templates make me uncomfortable, but how in the hell would you debug/test that? If all of that generates a few lines of assembly... :confused:
Charlie Gilley “They who can give up essential liberty to obtain a little temporary safety deserve neither liberty nor safety.” BF, 1759 Has never been more appropriate.
By running it through all the supported color models which would require crafting pixels of each different type implicit color models like RGB, Y'CbCr, C'MYk, HSV, grayscale/monochrome etc, and converting them back and forth to other models. The issue is that it's sometimes a lossy conversion so any test would have to account for that. I do not have unit tests for this function, but I've used it enough that I am confident in it. I did run several iterations of its use through godbolt.org to make sure it was producing the ASM i was expecting. Edit: To my mind the comments aren't all that lacking, at least in context. That functions structure is very simple, and most of the variable names are descriptive if abbreviated. I can look at it, with the comments that are there and tell A) exactly what it's doing and B) where to add things to get it to do more Any more than that, and it would create a comment maintenance issue, wherein i'm expressing intent twice, once through comments, and once through code, and creating double the maintenance, and the potential for comments to get out of sync. The issue is that my metadata functions and helpers aren't documented as much as they need to be
Check out my IoT graphics library here: https://honeythecodewitch/gfx
-
Forgive me for what I'm about to paste. The whole grotty mess is in the
convert<>()function at the bottom of this horrible file: gfx/include/gfx_pixel.hpp[^] I've omitted several pages of code. The worst/best part of this is it's all done at compile time, and generates at most several lines of asm, but often a lot less, and in cases where the input values are known at compile time the entire computation is done at compile time and generates exactly no binary code. Despite that, it is as if it was written at gunpoint. I could break it up into a ton of nasty routines but that just feels like spreading the mess around. I'm ready for the flaming. :~// converts a pixel to the destination pixel type
template
constexpr static inline gfx_result convert(PixelTypeLhs source,PixelTypeRhs* result,const PixelTypeRhs* background=nullptr) {
static_assert(helpers::is_same::value || !PixelTypeLhs::template has_channel_names::value,"left hand pixel must not be indexed");
static_assert(helpers::is_same::value || !PixelTypeRhs::template has_channel_names::value,"right hand pixel must not be indexed");
if(nullptr==result) return gfx_result::invalid_argument;
if(helpers::is_same::value) {
if(nullptr==background) {
result->native_value=source.native_value;
return gfx_result::success;
} else {
return gfx_result::invalid_format;
}
}
bool good = false;
PixelTypeRhs tmp;
typename PixelTypeRhs::int_type native_value = tmp.native_value;// here's where we gather color model information using is\_rgb = typename PixelTypeLhs::template has\_channel\_names; using is\_yuv = typename PixelTypeLhs::template has\_channel\_names; using is\_ycbcr = typename PixelTypeLhs::template has\_channel\_names; using is\_hsv = typename PixelTypeLhs::template has\_channel\_names; using is\_hsl = typename Pixehoney the codewitch wrote:
I'm ready for the flaming. :~
erm... read the red text at the top of the page :laugh:
“That which can be asserted without evidence, can be dismissed without evidence.”
― Christopher Hitchens
-
Forgive me for what I'm about to paste. The whole grotty mess is in the
convert<>()function at the bottom of this horrible file: gfx/include/gfx_pixel.hpp[^] I've omitted several pages of code. The worst/best part of this is it's all done at compile time, and generates at most several lines of asm, but often a lot less, and in cases where the input values are known at compile time the entire computation is done at compile time and generates exactly no binary code. Despite that, it is as if it was written at gunpoint. I could break it up into a ton of nasty routines but that just feels like spreading the mess around. I'm ready for the flaming. :~// converts a pixel to the destination pixel type
template
constexpr static inline gfx_result convert(PixelTypeLhs source,PixelTypeRhs* result,const PixelTypeRhs* background=nullptr) {
static_assert(helpers::is_same::value || !PixelTypeLhs::template has_channel_names::value,"left hand pixel must not be indexed");
static_assert(helpers::is_same::value || !PixelTypeRhs::template has_channel_names::value,"right hand pixel must not be indexed");
if(nullptr==result) return gfx_result::invalid_argument;
if(helpers::is_same::value) {
if(nullptr==background) {
result->native_value=source.native_value;
return gfx_result::success;
} else {
return gfx_result::invalid_format;
}
}
bool good = false;
PixelTypeRhs tmp;
typename PixelTypeRhs::int_type native_value = tmp.native_value;// here's where we gather color model information using is\_rgb = typename PixelTypeLhs::template has\_channel\_names; using is\_yuv = typename PixelTypeLhs::template has\_channel\_names; using is\_ycbcr = typename PixelTypeLhs::template has\_channel\_names; using is\_hsv = typename PixelTypeLhs::template has\_channel\_names; using is\_hsl = typename PixeIt works and has a sole responsibility. If it was to be refactored, you could use chain of responsibility to break it up.
Graeme
"I fear not the man who has practiced ten thousand kicks one time, but I fear the man that has practiced one kick ten thousand times!" - Bruce Lee
“I fear not the man who has practised 10,000 kicks once, but I fear the man who has practised one kick 10,000 times.” - Bruce Lee.
-
It works and has a sole responsibility. If it was to be refactored, you could use chain of responsibility to break it up.
Graeme
"I fear not the man who has practiced ten thousand kicks one time, but I fear the man that has practiced one kick ten thousand times!" - Bruce Lee
It would require duplicating a lot of code because most of it is compile time computations of the sort that aren't easily passed to another function, even at compile time. That's the other issue with breaking it up. There would need to be a large common preamble to each routine.
Check out my IoT graphics library here: https://honeythecodewitch.com/gfx
-
It would require duplicating a lot of code because most of it is compile time computations of the sort that aren't easily passed to another function, even at compile time. That's the other issue with breaking it up. There would need to be a large common preamble to each routine.
Check out my IoT graphics library here: https://honeythecodewitch.com/gfx
Ah, so for IOT, where memory is a premium, it's better not to. Totally understand. 😊
Graeme
"I fear not the man who has practiced ten thousand kicks one time, but I fear the man that has practiced one kick ten thousand times!" - Bruce Lee
-
By running it through all the supported color models which would require crafting pixels of each different type implicit color models like RGB, Y'CbCr, C'MYk, HSV, grayscale/monochrome etc, and converting them back and forth to other models. The issue is that it's sometimes a lossy conversion so any test would have to account for that. I do not have unit tests for this function, but I've used it enough that I am confident in it. I did run several iterations of its use through godbolt.org to make sure it was producing the ASM i was expecting. Edit: To my mind the comments aren't all that lacking, at least in context. That functions structure is very simple, and most of the variable names are descriptive if abbreviated. I can look at it, with the comments that are there and tell A) exactly what it's doing and B) where to add things to get it to do more Any more than that, and it would create a comment maintenance issue, wherein i'm expressing intent twice, once through comments, and once through code, and creating double the maintenance, and the potential for comments to get out of sync. The issue is that my metadata functions and helpers aren't documented as much as they need to be
Check out my IoT graphics library here: https://honeythecodewitch/gfx
honey the codewitch wrote:
I do not have unit tests for this function, but I've used it enough that I am confident in it.
I distrust any app in the hospital. If it works, ship it, right? And then you see a VB6 app starting at the beginning of the colonoscopy. I am pretty sure at that point that the person who wrote it has said that it has been used enough for his or her confidence.
honey the codewitch wrote:
The issue is that my metadata functions and helpers aren't documented as much as they need to be
Why not?
Bastard Programmer from Hell :suss: "If you just follow the bacon Eddy, wherever it leads you, then you won't have to think about politics." -- Some Bell.
-
honey the codewitch wrote:
I do not have unit tests for this function, but I've used it enough that I am confident in it.
I distrust any app in the hospital. If it works, ship it, right? And then you see a VB6 app starting at the beginning of the colonoscopy. I am pretty sure at that point that the person who wrote it has said that it has been used enough for his or her confidence.
honey the codewitch wrote:
The issue is that my metadata functions and helpers aren't documented as much as they need to be
Why not?
Bastard Programmer from Hell :suss: "If you just follow the bacon Eddy, wherever it leads you, then you won't have to think about politics." -- Some Bell.
Eddy Vluggen wrote:
I distrust any app in the hospital. If it works, ship it, right?
This isn't a medical application.
Eddy Vluggen wrote:
The issue is that my metadata functions and helpers aren't documented as much as they need to be
Because I am human, mortal, and have finite amounts of time and energy.
Check out my IoT graphics library here: https://honeythecodewitch.com/gfx And my IoT UI/User Experience library here: https://honeythecodewitch.com/uix
-
Eddy Vluggen wrote:
I distrust any app in the hospital. If it works, ship it, right?
This isn't a medical application.
Eddy Vluggen wrote:
The issue is that my metadata functions and helpers aren't documented as much as they need to be
Because I am human, mortal, and have finite amounts of time and energy.
Check out my IoT graphics library here: https://honeythecodewitch.com/gfx And my IoT UI/User Experience library here: https://honeythecodewitch.com/uix
honey the codewitch wrote:
Because I am human, mortal, and have finite amounts of time and energy.
What kind of excuse is that? :) Seriously, I would mostly prefer to have clear, maintainable code than a tour de force like you wrote, even at the cost of performance. The only exceptions are when the function is the one preventing the program from meeting some performance bar.
Freedom is the freedom to say that two plus two make four. If that is granted, all else follows. -- 6079 Smith W.
-
honey the codewitch wrote:
Because I am human, mortal, and have finite amounts of time and energy.
What kind of excuse is that? :) Seriously, I would mostly prefer to have clear, maintainable code than a tour de force like you wrote, even at the cost of performance. The only exceptions are when the function is the one preventing the program from meeting some performance bar.
Freedom is the freedom to say that two plus two make four. If that is granted, all else follows. -- 6079 Smith W.
My code is quite extensively documented, just not all the dark corners that only I end up needing anyway. Pixel metadata is one of those things. People don't really need to know how many bits exist to the left of an arbitrary pixel channel, and if they really do they can look at the comments in the source code.
Check out my IoT graphics library here: https://honeythecodewitch.com/gfx And my IoT UI/User Experience library here: https://honeythecodewitch.com/uix
-
Eddy Vluggen wrote:
I distrust any app in the hospital. If it works, ship it, right?
This isn't a medical application.
Eddy Vluggen wrote:
The issue is that my metadata functions and helpers aren't documented as much as they need to be
Because I am human, mortal, and have finite amounts of time and energy.
Check out my IoT graphics library here: https://honeythecodewitch.com/gfx And my IoT UI/User Experience library here: https://honeythecodewitch.com/uix
honey the codewitch wrote:
Because I am human, mortal, and have finite amounts of time and energy.
You, mortal? Erlkönig: programmer-and-the-devil.shtml[^]
Bastard Programmer from Hell :suss: "If you just follow the bacon Eddy, wherever it leads you, then you won't have to think about politics." -- Some Bell.
