Skip to content
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

[Core] Fix assumption of colour channel ranges as 0-255 #75

Open
mikejsavage opened this issue Dec 21, 2024 · 5 comments
Open

[Core] Fix assumption of colour channel ranges as 0-255 #75

mikejsavage opened this issue Dec 21, 2024 · 5 comments

Comments

@mikejsavage
Copy link
Contributor

Clay_Color is defined as 4 floats, so you would expect each component to range from 0-1, but the inspector hardcodes colours with components in 0-255 as if it was 4x uint8.

Besides the inspector clay doesn't seem to care what you put in Clay_Color so it's not a big deal, but it would still be nice to clear this up.

@raugl
Copy link

raugl commented Dec 23, 2024

I too would like to see it changed to 4 uint8_ts. I'm working on another set of bindings for zig and I have replaced clay's BoundingBox and Vector2 with raylib's equivalents as they allow the use of methods on otherwise memory compatible types. I would love to be able to do the same with Color, especially since I don't see a good reason for them being floats, maybe other than fewer internal casts.

@nicbarker
Copy link
Owner

@mikejsavage I totally agree, the inspector hardcoding colour values was a mistake. I wanted to make sure the data type was flexible enough to support formats that use 4x floats in 0-1 range, but they are 0-255 simply because that's what raylib asks for.

Perhaps the correct solution is that the user can optionally provide a function pointer that transforms Clay_Color -> Clay_Color that the debugger calls to transform it's internal colours into a format that the user decides on.

@nicbarker nicbarker changed the title Minor: Clay_Color range is unintuitive [Core] Fix assumption of colours as 0-255 Dec 24, 2024
@nicbarker nicbarker changed the title [Core] Fix assumption of colours as 0-255 [Core] Fix assumption of colours channel ranges as 0-255 Dec 24, 2024
@nicbarker nicbarker changed the title [Core] Fix assumption of colours channel ranges as 0-255 [Core] Fix assumption of colour channel ranges as 0-255 Dec 24, 2024
matevzmihalic added a commit to matevzmihalic/clay that referenced this issue Dec 26, 2024
@DiThi
Copy link

DiThi commented Jan 1, 2025

I think it would be better and more consistent to use colors 0 to 1 in linear space, and then leave it to the specific implementation (like raylib in this case) to convert them to uint8 in sRGB space or whatever. In my case I would use the colors in linear space directly, only converting everything to sRGB as the last step on the shader (or not even that if the target supports sRGB framebuffers). Also transitions and gradients look way better when interpolated in linear space.

To be compatible with existing code, you can auto detect the mode (0-255 or 0.0-1.0) if there's an alpha element over 1 and issue a deprecation warning.

@leidegre
Copy link

leidegre commented Jan 3, 2025

I ran into this issues as well, it's great that it just passes whatever you provide to the backend so it can do what it wants. This way there's no extra work. I'm using a custom Direct2D backend and it is using color values in the range 0,1. This works, the only trick I had to employ was to convert the debug color values. So far, this is the only component that made an assumption about what color to use but that is easy to fix yourself.

#define CLAY_COLOR_NORMALIZE(c,n) (Clay_Color){c.r/n,c.g/n,c.b/n,c.a/n}

CLAY__DEBUGVIEW_COLOR_1=CLAY_COLOR_NORMALIZE(CLAY__DEBUGVIEW_COLOR_1,255.0f);
CLAY__DEBUGVIEW_COLOR_2=CLAY_COLOR_NORMALIZE(CLAY__DEBUGVIEW_COLOR_2,255.0f);
CLAY__DEBUGVIEW_COLOR_3=CLAY_COLOR_NORMALIZE(CLAY__DEBUGVIEW_COLOR_3,255.0f);
CLAY__DEBUGVIEW_COLOR_4=CLAY_COLOR_NORMALIZE(CLAY__DEBUGVIEW_COLOR_4,255.0f);
CLAY__DEBUGVIEW_COLOR_SELECTED_ROW=CLAY_COLOR_NORMALIZE(CLAY__DEBUGVIEW_COLOR_SELECTED_ROW,255.0f);
Clay__DebugView_TextNameConfig.textColor=CLAY_COLOR_NORMALIZE(Clay__DebugView_TextNameConfig.textColor,255.0f);

These though, they are hardcoded, if we make globals of these as well you can easily convert them to whatever range suits your backend.

Clay__DebugElementConfigTypeLabelConfig Clay__DebugGetElementConfigTypeLabel(Clay__ElementConfigType type) {
    switch (type) {
        case CLAY__ELEMENT_CONFIG_TYPE_RECTANGLE: return CLAY__INIT(Clay__DebugElementConfigTypeLabelConfig) { CLAY_STRING("Rectangle"), {243,134,48,255} };
        case CLAY__ELEMENT_CONFIG_TYPE_TEXT: return CLAY__INIT(Clay__DebugElementConfigTypeLabelConfig) { CLAY_STRING("Text"), {105,210,231,255} };
        case CLAY__ELEMENT_CONFIG_TYPE_IMAGE: return CLAY__INIT(Clay__DebugElementConfigTypeLabelConfig) { CLAY_STRING("Image"), {121,189,154,255} };
        case CLAY__ELEMENT_CONFIG_TYPE_FLOATING_CONTAINER: return CLAY__INIT(Clay__DebugElementConfigTypeLabelConfig) { CLAY_STRING("Floating"), {250,105,0,255} };
        case CLAY__ELEMENT_CONFIG_TYPE_SCROLL_CONTAINER: return CLAY__INIT(Clay__DebugElementConfigTypeLabelConfig) { CLAY_STRING("Scroll"), {242,196,90,255} };
        case CLAY__ELEMENT_CONFIG_TYPE_BORDER_CONTAINER: return CLAY__INIT(Clay__DebugElementConfigTypeLabelConfig) { CLAY_STRING("Border"), {108,91,123, 255} };
        case CLAY__ELEMENT_CONFIG_TYPE_CUSTOM: return CLAY__INIT(Clay__DebugElementConfigTypeLabelConfig) { CLAY_STRING("Custom"), {11,72,107,255} };
        default: break;
    }
    return CLAY__INIT(Clay__DebugElementConfigTypeLabelConfig) { CLAY_STRING("Error"), {0,0,0,255} };
}

@leidegre
Copy link

leidegre commented Jan 3, 2025

I made a PR with some suggestions that make the debug view work with color values in any range.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants