SFML / imgui-sfml

Dear ImGui backend for use with SFML
MIT License
1.08k stars 163 forks source link

refactor: fix conversion warnings on Clang #245

Closed JohelEGP closed 11 months ago

JohelEGP commented 11 months ago

Previously:

/home/johel/Documents/C++/Forks/SFML/imgui-sfml/imgui-SFML.cpp:571:66: warning: implicit conversion from 'const unsigned int' to 'float' may lose precision [-Wimplicit-int-float-conversion]
  571 |             io.DisplaySize = ImVec2(event.size.width, event.size.height);
      |                              ~~~~~~                   ~~~~~~~~~~~^~~~~~
/home/johel/Documents/C++/Forks/SFML/imgui-sfml/imgui-SFML.cpp:571:48: warning: implicit conversion from 'const unsigned int' to 'float' may lose precision [-Wimplicit-int-float-conversion]
  571 |             io.DisplaySize = ImVec2(event.size.width, event.size.height);
      |                              ~~~~~~ ~~~~~~~~~~~^~~~~
/home/johel/Documents/C++/Forks/SFML/imgui-sfml/imgui-SFML.cpp:574:68: warning: implicit conversion from 'const int' to 'float' may lose precision [-Wimplicit-int-float-conversion]
  574 |             io.AddMousePosEvent(event.mouseMove.x, event.mouseMove.y);
      |                ~~~~~~~~~~~~~~~~                    ~~~~~~~~~~~~~~~~^
/home/johel/Documents/C++/Forks/SFML/imgui-sfml/imgui-SFML.cpp:574:49: warning: implicit conversion from 'const int' to 'float' may lose precision [-Wimplicit-int-float-conversion]
  574 |             io.AddMousePosEvent(event.mouseMove.x, event.mouseMove.y);
      |                ~~~~~~~~~~~~~~~~ ~~~~~~~~~~~~~~~~^
/home/johel/Documents/C++/Forks/SFML/imgui-sfml/imgui-SFML.cpp:803:45: warning: implicit conversion changes signedness: 'int' to 'unsigned int' [-Wsign-conversion]
  803 |     if (!texture.create(sf::Vector2u(width, height))) {
      |                         ~~                  ^~~~~~
/home/johel/Documents/C++/Forks/SFML/imgui-sfml/imgui-SFML.cpp:803:38: warning: implicit conversion changes signedness: 'int' to 'unsigned int' [-Wsign-conversion]
  803 |     if (!texture.create(sf::Vector2u(width, height))) {
      |                         ~~           ^~~~~
/home/johel/Documents/C++/Forks/SFML/imgui-sfml/imgui-SFML.cpp:992:62: warning: implicit conversion from 'const int' to 'float' may lose precision [-Wimplicit-int-float-conversion]
  992 |     ImVec2 uv0(textureRect.left / textureSize.x, textureRect.top / textureSize.y);
      |                                                  ~~~~~~~~~~~~^~~ ~
/home/johel/Documents/C++/Forks/SFML/imgui-sfml/imgui-SFML.cpp:992:28: warning: implicit conversion from 'const int' to 'float' may lose precision [-Wimplicit-int-float-conversion]
  992 |     ImVec2 uv0(textureRect.left / textureSize.x, textureRect.top / textureSize.y);
      |                ~~~~~~~~~~~~^~~~ ~
/home/johel/Documents/C++/Forks/SFML/imgui-sfml/imgui-SFML.cpp:994:33: warning: implicit conversion from 'int' to 'float' may lose precision [-Wimplicit-int-float-conversion]
  994 |                (textureRect.top + textureRect.height) / textureSize.y);
      |                 ~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~  ~
/home/johel/Documents/C++/Forks/SFML/imgui-sfml/imgui-SFML.cpp:993:34: warning: implicit conversion from 'int' to 'float' may lose precision [-Wimplicit-int-float-conversion]
  993 |     ImVec2 uv1((textureRect.left + textureRect.width) / textureSize.x,
      |                 ~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~  ~
/home/johel/Documents/C++/Forks/SFML/imgui-sfml/imgui-SFML.cpp:1057:62: warning: implicit conversion from 'const int' to 'float' may lose precision [-Wimplicit-int-float-conversion]
 1057 |     ImVec2 uv0(textureRect.left / textureSize.x, textureRect.top / textureSize.y);
      |                                                  ~~~~~~~~~~~~^~~ ~
/home/johel/Documents/C++/Forks/SFML/imgui-sfml/imgui-SFML.cpp:1057:28: warning: implicit conversion from 'const int' to 'float' may lose precision [-Wimplicit-int-float-conversion]
 1057 |     ImVec2 uv0(textureRect.left / textureSize.x, textureRect.top / textureSize.y);
      |                ~~~~~~~~~~~~^~~~ ~
/home/johel/Documents/C++/Forks/SFML/imgui-sfml/imgui-SFML.cpp:1059:33: warning: implicit conversion from 'int' to 'float' may lose precision [-Wimplicit-int-float-conversion]
 1059 |                (textureRect.top + textureRect.height) / textureSize.y);
      |                 ~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~  ~
/home/johel/Documents/C++/Forks/SFML/imgui-sfml/imgui-SFML.cpp:1058:34: warning: implicit conversion from 'int' to 'float' may lose precision [-Wimplicit-int-float-conversion]
 1058 |     ImVec2 uv1((textureRect.left + textureRect.width) / textureSize.x,
      |                 ~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~  ~
/home/johel/Documents/C++/Forks/SFML/imgui-sfml/imgui-SFML.cpp:1242:61: warning: implicit conversion from 'int' to 'float' may lose precision [-Wimplicit-int-float-conversion]
 1242 |                 if (clip_rect.x < fb_width && clip_rect.y < fb_height && clip_rect.z >= 0.0f &&
      |                                                           ~ ^~~~~~~~~
/home/johel/Documents/C++/Forks/SFML/imgui-sfml/imgui-SFML.cpp:1242:35: warning: implicit conversion from 'int' to 'float' may lose precision [-Wimplicit-int-float-conversion]
 1242 |                 if (clip_rect.x < fb_width && clip_rect.y < fb_height && clip_rect.z >= 0.0f &&
      |                                 ~ ^~~~~~~~
/home/johel/Documents/C++/Forks/SFML/imgui-sfml/imgui-SFML.cpp:1245:55: warning: implicit conversion from 'int' to 'float' may lose precision [-Wimplicit-int-float-conversion]
 1245 |                     glScissor((int)clip_rect.x, (int)(fb_height - clip_rect.w),
      |                                                       ^~~~~~~~~ ~
/home/johel/Documents/C++/Forks/SFML/imgui-sfml/imgui-SFML.cpp:1275:18: warning: implicit conversion changes signedness: 'GLint' (aka 'int') to 'GLenum' (aka 'unsigned int') [-Wsign-conversion]
 1275 |     glShadeModel(last_shade_model);
      |     ~~~~~~~~~~~~ ^~~~~~~~~~~~~~~~
/home/johel/Documents/C++/Forks/SFML/imgui-sfml/imgui-SFML.cpp:1331:89: warning: implicit conversion changes signedness: 'int' to 'unsigned int' [-Wsign-conversion]
 1331 |             bool isPressed = sf::Joystick::isButtonPressed(s_currWindowCtx->joystickId, i);
      |                              ~~                                                         ^
19 warnings generated.
oprypin commented 11 months ago

Could you explain how you're getting these warnings? I could not observe them no matter what I tried, for example:

$ clang++ -Werror -Wall -Wextra -I. -I../sfml/include -I../cimgui/imgui examples/minimal/main.cpp imgui-SFML.cpp -c

$ CC=clang CXX=clang++ cmake -DCMAKE_CXX_FLAGS="-Werror -Wall -Wextra" . -DIMGUI_DIR=../imgui -DSFML_DIR=../sfml && cmake --build .
-- Found SFML 2.6.0 in ../sfml
-- Found ImGui v1.89.8 WIP in ../imgui
oprypin commented 11 months ago
$ clang++ --version                                                                                                
clang version 15.0.7
Target: x86_64-pc-linux-gnu
JohelEGP commented 11 months ago

It's probably -Wconversion.

oprypin commented 11 months ago

Ah yes thanks. Needed both -Wconversion -Wsign-conversion

oprypin commented 11 months ago

Hm I think the float conversion ones are totally harmless. Up to 224, float covers int. And it's not like making these places explicit gives us any insights.

Perhaps fixing and enforcing -Wsign-conversion is a good goal, but indeed how to actually enforce them?

JohelEGP commented 11 months ago

I'm using Clang 17, and my command lines don't have -Wsign-conversion. So I'm guessing its effects were integrated into -Wconversion.

JohelEGP commented 11 months ago

Perhaps fixing and enforcing -Wsign-conversion is a good goal, but indeed how to actually enforce them?

The way I went about it was doing what others did: https://github.com/search?q=repo%3ASFML%2Fimgui-sfml+warnings&type=pullrequests.

ChrisThrasher commented 11 months ago

Coincidentally I also have a compiler warning related PR but this time I'm actually adding the warnings to the build script so we don't regress with compiler warnings. See #246.

ChrisThrasher commented 11 months ago

https://github.com/SFML/imgui-sfml/commit/92dd79ccd426858787fe4384fa24892fba3dfac5

I started fixing these conversion warnings. MSVC caught a few to begin with.