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

Custom border width in Image() #8131

Closed
wants to merge 1 commit into from
Closed

Conversation

micb25
Copy link
Contributor

@micb25 micb25 commented Nov 5, 2024

This PR introduces the possibility to specify a custom border width in Image() by adding an additional parameter border_width. There is currently no way to adjust the width of a drawn image border as it is hard-coded to 1 pixel, which may be insufficient for high DPI displays.

Verified

This commit was signed with the committer’s verified signature.
nicolo-ribaudo Nicolò Ribaudo
@ocornut ocornut added the style label Nov 6, 2024
@ocornut
Copy link
Owner

ocornut commented Nov 6, 2024

Hello, thanks for the PR. I might need to put this one on hold for a bit.
It may be better to add a float ImageBorderSize to the style structure.
(I'm not even sure border_col should be an argument to this function tbh)

@micb25
Copy link
Contributor Author

micb25 commented Nov 6, 2024

Thanks, I fully agree.

In the current implementation the border size gets set via the alpha channel of the border color. I think this logic is somewhat inappropriate, because the total image size depends on the alpha channel. E.g., I have a window with several image previews and I want to highlight the active one by drawing an image border, however, this leads to different sizes that can only be fixed by setting a small non-zero alpha channel.

Thus, I would prefer two new styles (border color and border width). What do you think?

@ocornut
Copy link
Owner

ocornut commented Jan 21, 2025

Also linking to #8238 which mention the same issue.

ocornut added a commit that referenced this pull request Feb 27, 2025

Verified

This commit was signed with the committer’s verified signature.
nicolo-ribaudo Nicolò Ribaudo
…d ImageWithBg(). Added style.ImageBorderSize, ImGuiStyleVar_ImageBorderSize. (#8131, #8238)

Displaying a black background behind Font Atlas texture.
ocornut added a commit that referenced this pull request Feb 27, 2025

Verified

This commit was signed with the committer’s verified signature.
nicolo-ribaudo Nicolò Ribaudo
Amend 494ea57
@ocornut
Copy link
Owner

ocornut commented Feb 27, 2025

I have pushed a slightly larger refactor 494ea57 with solves this.

Image: removed 'tint_col' and 'border_col' parameter from Image() function.

  • Old function signature:
    void Image (ImTextureID tex_id, ImVec2 image_size, ImVec2 uv0 = (0,0), ImVec2 uv1 = (1,1), ImVec4 tint_col = (1,1,1,1), ImVec4 border_col = (0,0,0,0));
  • New function signatures:
    void Image (ImTextureID tex_id, ImVec2 image_size, ImVec2 uv0 = (0,0), ImVec2 uv1 = (1,1));
    void ImageWithBg(ImTextureID tex_id, ImVec2 image_size, ImVec2 uv0 = (0,0), ImVec2 uv1 = (1,1), ImVec4 bg_col = (0,0,0,0), ImVec4 tint_col = (1,1,1,1));

TL;DR: 'border_col' had misleading side-effect on layout, 'bg_col' was missing, parameter order couldn't be consistent with ImageButton().

  • New behavior always use ImGuiCol_Border color + style.ImageBorderSize / ImGuiStyleVar_ImageBorderSize.
  • Old behavior altered border size (and therefore layout) based on border color's alpha, which caused variety of problems.
  • Old behavior used a fixed value of 1.0f for border size which was not tweakable.
  • Kept legacy signature (will obsolete), which mimics the old behavior, but uses Max(1.0f, style.ImageBorderSize) when border_col is specified.
  • Added ImageWithBg() function which has both bg_col (which was missing and is more useful) and tint_col.
  • It was impossible to add 'bg_col' to Image() with a parameter order consistent with other functions, so we decided to remove tint_col and introduce ImageWithBg() which has both and in the right order.

@ocornut ocornut closed this Feb 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants