Fix error: conversion from ‘int’ to ‘uchar’ {aka ‘unsigned char’} #120879

Giang-Chaebol · 2024-04-21T15:14:53+02:00

Giang-Chaebol commented

2024-04-21 15:14:53 +02:00

This pull request fixes the following errors when following this instruction:

blender/source/blender/blenfont/intern/blf_glyph.cc:306:64: error: conversion from ‘int’ to ‘uchar’ {aka ‘unsigned char’} may change value [-Werror=conversion]
  306 |         g->bitmap[i] = blf_glyph_gamma(glyph->bitmap.buffer[i] * scale);

This pull request fixes the following errors when following [this](https://developer.blender.org/docs/handbook/building_blender/linux/#update-and-build) instruction: ``` blender/source/blender/blenfont/intern/blf_glyph.cc:306:64: error: conversion from ‘int’ to ‘uchar’ {aka ‘unsigned char’} may change value [-Werror=conversion] 306 | g->bitmap[i] = blf_glyph_gamma(glyph->bitmap.buffer[i] * scale); ```

Giang-Chaebol added 1 commit 2024-04-21 15:15:02 +02:00

2c80afa57b Fix error: conversion from ‘int’ to ‘uchar’ {aka ‘unsigned char’}

Iliya Katushenock requested review from Harley Acheson 2024-04-21 15:15:41 +02:00

Giang-Chaebol referenced this pull request

2024-04-21 15:31:59 +02:00

Fix conversions #120873

Harley Acheson commented

2024-04-25 18:36:15 +02:00

Hello, and thanks for looking at this. My compiler is not giving me this same warning, so I am a bit blind here.

This warning is about an assignment to uchar from a function that returns uchar. That function takes one uchar argument and we are giving it the product of multiplying a uchar with a char.

So to my eye your proposed solution of a functional "char" cast, should actually be a functional "uchar" cast? But isn't the real problem that "scale", defined a few lines earlier, should be defined as uchar?

Does the following remove this warning for you?

diff --git a/source/blender/blenfont/intern/blf_glyph.cc b/source/blender/blenfont/intern/blf_glyph.cc
index 471b0efe046..bdba076277c 100644
--- a/source/blender/blenfont/intern/blf_glyph.cc
+++ b/source/blender/blenfont/intern/blf_glyph.cc
@@ -281,11 +281,11 @@ static GlyphBLF *blf_glyph_cache_add_glyph(FontBLF *font,
              FT_PIXEL_MODE_GRAY,
              FT_PIXEL_MODE_GRAY2,
              FT_PIXEL_MODE_GRAY4))
     {
       /* Scale 1, 2, 4-bit gray to 8-bit. */
-      const char scale = char(255 / (glyph->bitmap.num_grays - 1));
+      const uchar scale = uchar(255 / (glyph->bitmap.num_grays - 1));
       for (int i = 0; i < buffer_size; i++) {
 #ifdef BLF_GAMMA_CORRECT_GLYPHS
         /* Convert coverage amounts to perceptually-improved lightness values. */
         g->bitmap[i] = blf_glyph_gamma(glyph->bitmap.buffer[i] * scale);
 #else

Hello, and thanks for looking at this. My compiler is not giving me this same warning, so I am a bit blind here. This warning is about an assignment to uchar from a function that returns uchar. That function takes one uchar argument and we are giving it the product of multiplying a uchar with a char. So to my eye your proposed solution of a functional "char" cast, should actually be a functional "uchar" cast? But isn't the real problem that "scale", defined a few lines earlier, should be defined as uchar? Does the following remove this warning for you? ```Diff diff --git a/source/blender/blenfont/intern/blf_glyph.cc b/source/blender/blenfont/intern/blf_glyph.cc index 471b0efe046..bdba076277c 100644 --- a/source/blender/blenfont/intern/blf_glyph.cc +++ b/source/blender/blenfont/intern/blf_glyph.cc @@ -281,11 +281,11 @@ static GlyphBLF *blf_glyph_cache_add_glyph(FontBLF *font, FT_PIXEL_MODE_GRAY, FT_PIXEL_MODE_GRAY2, FT_PIXEL_MODE_GRAY4)) { /* Scale 1, 2, 4-bit gray to 8-bit. */ - const char scale = char(255 / (glyph->bitmap.num_grays - 1)); + const uchar scale = uchar(255 / (glyph->bitmap.num_grays - 1)); for (int i = 0; i < buffer_size; i++) { #ifdef BLF_GAMMA_CORRECT_GLYPHS /* Convert coverage amounts to perceptually-improved lightness values. */ g->bitmap[i] = blf_glyph_gamma(glyph->bitmap.buffer[i] * scale); #else ```

Giang-Chaebol commented

2024-04-25 23:10:17 +02:00

Hello, and thanks for looking at this. My compiler is not giving me this same warning, so I am a bit blind here.

This warning is about an assignment to uchar from a function that returns uchar. That function takes one uchar argument and we are giving it the product of multiplying a uchar with a char.

So to my eye your proposed solution of a functional "char" cast, should actually be a functional "uchar" cast? But isn't the real problem that "scale", defined a few lines earlier, should be defined as uchar?

Does the following remove this warning for you?
diff --git a/source/blender/blenfont/intern/blf_glyph.cc b/source/blender/blenfont/intern/blf_glyph.cc
index 471b0efe046..bdba076277c 100644
--- a/source/blender/blenfont/intern/blf_glyph.cc
+++ b/source/blender/blenfont/intern/blf_glyph.cc
@@ -281,11 +281,11 @@ static GlyphBLF *blf_glyph_cache_add_glyph(FontBLF *font,
              FT_PIXEL_MODE_GRAY,
              FT_PIXEL_MODE_GRAY2,
              FT_PIXEL_MODE_GRAY4))
     {
       /* Scale 1, 2, 4-bit gray to 8-bit. */
-      const char scale = char(255 / (glyph->bitmap.num_grays - 1));
+      const uchar scale = uchar(255 / (glyph->bitmap.num_grays - 1));
       for (int i = 0; i < buffer_size; i++) {
 #ifdef BLF_GAMMA_CORRECT_GLYPHS
         /* Convert coverage amounts to perceptually-improved lightness values. */
         g->bitmap[i] = blf_glyph_gamma(glyph->bitmap.buffer[i] * scale);
 #else

It still gives me the error, see the video.

> Hello, and thanks for looking at this. My compiler is not giving me this same warning, so I am a bit blind here. > > This warning is about an assignment to uchar from a function that returns uchar. That function takes one uchar argument and we are giving it the product of multiplying a uchar with a char. > > So to my eye your proposed solution of a functional "char" cast, should actually be a functional "uchar" cast? But isn't the real problem that "scale", defined a few lines earlier, should be defined as uchar? > > Does the following remove this warning for you? > > ```Diff > diff --git a/source/blender/blenfont/intern/blf_glyph.cc b/source/blender/blenfont/intern/blf_glyph.cc > index 471b0efe046..bdba076277c 100644 > --- a/source/blender/blenfont/intern/blf_glyph.cc > +++ b/source/blender/blenfont/intern/blf_glyph.cc > @@ -281,11 +281,11 @@ static GlyphBLF *blf_glyph_cache_add_glyph(FontBLF *font, > FT_PIXEL_MODE_GRAY, > FT_PIXEL_MODE_GRAY2, > FT_PIXEL_MODE_GRAY4)) > { > /* Scale 1, 2, 4-bit gray to 8-bit. */ > - const char scale = char(255 / (glyph->bitmap.num_grays - 1)); > + const uchar scale = uchar(255 / (glyph->bitmap.num_grays - 1)); > for (int i = 0; i < buffer_size; i++) { > #ifdef BLF_GAMMA_CORRECT_GLYPHS > /* Convert coverage amounts to perceptually-improved lightness values. */ > g->bitmap[i] = blf_glyph_gamma(glyph->bitmap.buffer[i] * scale); > #else > > ``` > > > It still gives me the error, see the video. ![image](/attachments/9ea178a8-1747-46ae-be70-7c8dbbbee100)

BlenderError.mp4

4.0 MiB

image.png

1.2 MiB

Harley Acheson commented

2024-04-25 23:20:41 +02:00

It still gives me the error, see the video.

Sorry, but I'm not seeing the error in that video. Although you have blf_glyph.cc loaded, the capture is showing terminal messages to do with index_mask.cc

> It still gives me the error, see the video. Sorry, but I'm not seeing the error in that video. Although you have blf_glyph.cc loaded, the capture is showing terminal messages to do with index_mask.cc

Giang-Chaebol commented

2024-04-26 00:19:00 +02:00

It still gives me the error, see the video.

Sorry, but I'm not seeing the error in that video. Although you have blf_glyph.cc loaded, the capture is showing terminal messages to do with index_mask.cc

Yes sorry my bad. Hereby I send you the correct one.

> > It still gives me the error, see the video. > > Sorry, but I'm not seeing the error in that video. Although you have blf_glyph.cc loaded, the capture is showing terminal messages to do with index_mask.cc Yes sorry my bad. Hereby I send you the correct one. ![image](/attachments/1576b5df-d64e-4f7c-99b5-d1e70a0ee320)

BlenderError.mp4

5.3 MiB

image.png

1.2 MiB

Campbell Barton commented

2024-04-26 01:31:23 +02:00

@Giang-Chaebol it's strange that your running into these errors as I don't get them with recent Linux/macOS build environments, what compiler version are you using?

Campbell Barton commented

2024-04-26 01:43:22 +02:00

In the build output there is the message "unrecognized command line option" this is a hint that there is something wrong with the build as CMake is meant to detect this flag and only pass it in when it's supported (see: C_WARN_NO_STRINGOP_OVERREAD, CXX_WARN_NO_STRINGOP_OVERREAD).

Could you check if creating a new build directory resolves the issue?

In the build output there is the message "unrecognized command line option" this is a hint that there is something wrong with the build as CMake is meant to detect this flag and only pass it in when it's supported (see: C_WARN_NO_STRINGOP_OVERREAD, CXX_WARN_NO_STRINGOP_OVERREAD). Could you check if creating a new build directory resolves the issue?

Giang-Chaebol commented

2024-04-26 09:45:41 +02:00

@Giang-Chaebol it's strange that your running into these errors as I don't get them with recent Linux/macOS build environments, what compiler version are you using?

(multiverse) giangnguyen@giangnguyen-PC:~$ g++ --version
g++ (Ubuntu 9.4.0-1ubuntu1~20.04.2) 9.4.0
Copyright (C) 2019 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

(multiverse) giangnguyen@giangnguyen-PC:~$ cmake --version
cmake version 3.29.2

CMake suite maintained and supported by Kitware (kitware.com/cmake).
(multiverse) giangnguyen@giangnguyen-PC:~$ make --version
GNU Make 4.2.1
Built for x86_64-pc-linux-gnu
Copyright (C) 1988-2016 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.

> @Giang-Chaebol it's strange that your running into these errors as I don't get them with recent Linux/macOS build environments, what compiler version are you using? ``` (multiverse) giangnguyen@giangnguyen-PC:~$ g++ --version g++ (Ubuntu 9.4.0-1ubuntu1~20.04.2) 9.4.0 Copyright (C) 2019 Free Software Foundation, Inc. This is free software; see the source for copying conditions. There is NO warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. (multiverse) giangnguyen@giangnguyen-PC:~$ cmake --version cmake version 3.29.2 CMake suite maintained and supported by Kitware (kitware.com/cmake). (multiverse) giangnguyen@giangnguyen-PC:~$ make --version GNU Make 4.2.1 Built for x86_64-pc-linux-gnu Copyright (C) 1988-2016 Free Software Foundation, Inc. License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html> This is free software: you are free to change and redistribute it. There is NO WARRANTY, to the extent permitted by law. ```

Giang-Chaebol commented

2024-04-26 09:46:31 +02:00

In the build output there is the message "unrecognized command line option" this is a hint that there is something wrong with the build as CMake is meant to detect this flag and only pass it in when it's supported (see: C_WARN_NO_STRINGOP_OVERREAD, CXX_WARN_NO_STRINGOP_OVERREAD).

Could you check if creating a new build directory resolves the issue?

I always build from scratch, I even try to build it in a docker image and the same errors still appear.

> In the build output there is the message "unrecognized command line option" this is a hint that there is something wrong with the build as CMake is meant to detect this flag and only pass it in when it's supported (see: C_WARN_NO_STRINGOP_OVERREAD, CXX_WARN_NO_STRINGOP_OVERREAD). > > Could you check if creating a new build directory resolves the issue? I always build from scratch, I even try to build it in a docker image and the same errors still appear.

Campbell Barton commented

2024-04-26 09:51:51 +02:00

@Giang-Chaebol building shouldn't even have started since CMake checks GCC is at least 11.0.0, did you modify CMakeLists.txt in order to get Blender building?

What does gcc --version report?

@Giang-Chaebol building shouldn't even have started since CMake checks GCC is at least 11.0.0, did you modify `CMakeLists.txt` in order to get Blender building? What does `gcc --version` report?

Giang-Chaebol commented

2024-04-26 09:54:34 +02:00

@Giang-Chaebol building shouldn't even have started since CMake checks GCC is at least 11.0.0, did you modify CMakeLists.txt in order to get Blender building?

What does gcc --version report?

(multiverse) giangnguyen@giangnguyen-PC:~$ gcc --version
gcc (Ubuntu 11.4.0-2ubuntu1~20.04) 11.4.0
Copyright (C) 2021 Free Software Foundation, Inc.
This is free software; see the source for copying conditions. There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

> @Giang-Chaebol building shouldn't even have started since CMake checks GCC is at least 11.0.0, did you modify `CMakeLists.txt` in order to get Blender building? > > What does `gcc --version` report? (multiverse) giangnguyen@giangnguyen-PC:~$ gcc --version gcc (Ubuntu 11.4.0-2ubuntu1~20.04) 11.4.0 Copyright (C) 2021 Free Software Foundation, Inc. This is free software; see the source for copying conditions. There is NO warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

Giang-Chaebol commented

2024-04-26 09:55:57 +02:00

@Giang-Chaebol building shouldn't even have started since CMake checks GCC is at least 11.0.0, did you modify CMakeLists.txt in order to get Blender building?

What does gcc --version report?

And no, I don't modify anything.

> @Giang-Chaebol building shouldn't even have started since CMake checks GCC is at least 11.0.0, did you modify `CMakeLists.txt` in order to get Blender building? > > What does `gcc --version` report? And no, I don't modify anything.

Campbell Barton referenced this issue from a commit

2024-04-26 10:02:40 +02:00

CMake: check both C & C++ compiler versions

Campbell Barton commented

2024-04-26 10:04:06 +02:00

@Giang-Chaebol I've committed additional checks for C++ version e5dfc814e9, can you check that the build now raises an error early on?

@Giang-Chaebol I've committed additional checks for C++ version e5dfc814e9c46f35c12fc7638bb6926d684dd2a3, can you check that the build now raises an error early on?

Giang-Chaebol commented

2024-04-26 12:19:15 +02:00

@Giang-Chaebol I've committed additional checks for C++ version e5dfc814e9, can you check that the build now raises an error early on?

Ok the error from your CMakeLists.txt appeared to stop me from building it. I upgraded my g++ to 11 and rebuilt it. Now it can compile fine without giving me any errors.

Btw, your message is confusing:

if("${CMAKE_CXX_COMPILER_VERSION}" VERSION_LESS "11.0.0")
    message(FATAL_ERROR "\
The minimum supported version of GCC is 11.0.0, found ${CMAKE_CXX_COMPILER_VERSION}"
    )
  endif()

instead of GCC, it should be G++, otherwise it doesn't make sense.

> @Giang-Chaebol I've committed additional checks for C++ version e5dfc814e9c46f35c12fc7638bb6926d684dd2a3, can you check that the build now raises an error early on? Ok the error from your `CMakeLists.txt` appeared to stop me from building it. I upgraded my `g++` to 11 and rebuilt it. Now it can compile fine without giving me any errors. Btw, your message is confusing: ``` if("${CMAKE_CXX_COMPILER_VERSION}" VERSION_LESS "11.0.0") message(FATAL_ERROR "\ The minimum supported version of GCC is 11.0.0, found ${CMAKE_CXX_COMPILER_VERSION}" ) endif() ``` instead of GCC, it should be G++, otherwise it doesn't make sense.

Giang-Chaebol commented

2024-04-26 12:28:46 +02:00

However, I still think it might be better to fix the line for cleaner code. Explicit type conversion prevents data loss and increases clarity.

Campbell Barton commented

2024-04-26 13:39:30 +02:00

Btw, your message is confusing:
if("${CMAKE_CXX_COMPILER_VERSION}" VERSION_LESS "11.0.0")
    message(FATAL_ERROR "\
The minimum supported version of GCC is 11.0.0, found ${CMAKE_CXX_COMPILER_VERSION}"
    )
  endif()
instead of GCC, it should be G++, otherwise it doesn't make sense.

GCC is the name of the project, g++ is the command but this is still typically referred to as the GCC version.

Clarified between C/C++ compilers 5f6516a19a.

> Btw, your message is confusing: > > ``` > if("${CMAKE_CXX_COMPILER_VERSION}" VERSION_LESS "11.0.0") > message(FATAL_ERROR "\ > The minimum supported version of GCC is 11.0.0, found ${CMAKE_CXX_COMPILER_VERSION}" > ) > endif() > ``` > > instead of GCC, it should be G++, otherwise it doesn't make sense. GCC is the name of the project, `g++` is the command but this is still typically referred to as the GCC version. Clarified between C/C++ compilers 5f6516a19acd821b9bfb37321bfa257b8cca323d.

👍 1

Campbell Barton commented

2024-04-26 13:40:24 +02:00

Closing this PR as the error was caused by an unsupported C++ compiler.

Our CMake compiler checks have been updated so this wont happen again.

Closing this PR as the error was caused by an unsupported C++ compiler. Our CMake compiler checks have been updated so this wont happen again.

🎉 2

Campbell Barton closed this pull request

2024-04-26 13:40:27 +02:00

Pull request closed

Please reopen this pull request to perform a merge.

Sign in to join this conversation.

No reviewers

No Label

Download

What's New

Blender Studio

Manual

Developers Blog

Documentation

Benchmark

Blender Conference

Development Fund

One-time Donations

Fix error: conversion from ‘int’ to ‘uchar’ {aka ‘unsigned char’} #120879

Pull request closed