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

Fix improve the octree quantization method using better Euclidean color distance (fix #2787) #4416

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

Gasparoken
Copy link
Member

First approach.

I'm still looking for a way to run the color mode conversion preview when changing the "color fit criteria" selector (combo box), just like it runs when changing the RGB mapping method.

fix #2787

@Gasparoken Gasparoken self-assigned this Apr 11, 2024
@Gasparoken Gasparoken requested a review from dacap as a code owner April 11, 2024 22:22
Copy link
Collaborator

@aseprite-bot aseprite-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clang-tidy made some suggestions

@@ -73,7 +73,8 @@ SetPixelFormat::SetPixelFormat(Sprite* sprite,
const render::Dithering& dithering,
const doc::RgbMapAlgorithm mapAlgorithm,
doc::rgba_to_graya_func toGray,
render::TaskDelegate* delegate)
render::TaskDelegate* delegate,
const doc::FitCriteria fitCriteria)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: parameter 'fitCriteria' is unused [misc-unused-parameters]

Suggested change
const doc::FitCriteria fitCriteria)
const doc::FitCriteria /*fitCriteria*/)

@@ -37,7 +38,8 @@ namespace cmd {
const render::Dithering& dithering,
const doc::RgbMapAlgorithm mapAlgorithm,
doc::rgba_to_graya_func toGray,
render::TaskDelegate* delegate);
render::TaskDelegate* delegate,
const doc::FitCriteria fitCriteria = doc::FitCriteria::DEFAULT);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: parameter 'fitCriteria' is const-qualified in the function declaration; const-qualification of parameters only has an effect in function definitions [readability-avoid-const-params-in-decls]

Suggested change
const doc::FitCriteria fitCriteria = doc::FitCriteria::DEFAULT);
doc::FitCriteria fitCriteria = doc::FitCriteria::DEFAULT);

@@ -56,7 +58,8 @@
const bool isBackground,
const doc::RgbMapAlgorithm mapAlgorithm,
doc::rgba_to_graya_func toGray,
render::TaskDelegate* delegate);
render::TaskDelegate* delegate,
const doc::FitCriteria fitCriteria = doc::FitCriteria::DEFAULT);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: parameter 'fitCriteria' is const-qualified in the function declaration; const-qualification of parameters only has an effect in function definitions [readability-avoid-const-params-in-decls]

Suggested change
const doc::FitCriteria fitCriteria = doc::FitCriteria::DEFAULT);
doc::FitCriteria fitCriteria = doc::FitCriteria::DEFAULT);

src/app/commands/cmd_change_pixel_format.cpp Show resolved Hide resolved
@@ -35,19 +35,21 @@ void OctreeNode::addColor(color_t c, int level, OctreeNode* parent,
(*m_children)[index].addColor(c, level + 1, this, paletteIndex, levelDeep);
}

int OctreeNode::mapColor(int r, int g, int b, int a, int mask_index, const Palette* palette, int level) const
int OctreeNode::mapColor(int r, int g, int b, int a, int mask_index,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: function 'mapColor' is within a recursive call chain [misc-no-recursion]

int OctreeNode::mapColor(int  r, int g, int b, int a, int mask_index,
                ^
Additional context

src/doc/octree_map.cpp:37: example recursive call chain, starting from function 'mapColor'

int OctreeNode::mapColor(int  r, int g, int b, int a, int mask_index,
                ^

src/doc/octree_map.cpp:51: Frame #1: function 'mapColor' calls function 'mapColor' here:

  return (*m_children)[index].mapColor(r, g, b, a, mask_index, palette, level + 1, fitCriteria);
         ^

src/doc/octree_map.cpp:51: ... which was the starting point of the recursive call chain; there may be other cycles

  return (*m_children)[index].mapColor(r, g, b, a, mask_index, palette, level + 1, fitCriteria);
         ^

else
*dst_it = palette->findBestfit(c, c, c, a, new_mask_color);
*dst_it = palette->findBestfit(c, c, c, a, new_mask_color, fitCriteria);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: narrowing conversion from 'color_t' (aka 'unsigned int') to signed type 'int' is implementation-defined [bugprone-narrowing-conversions]

              *dst_it = palette->findBestfit(c, c, c, a, new_mask_color, fitCriteria);
                                                ^

else
*dst_it = palette->findBestfit(c, c, c, a, new_mask_color);
*dst_it = palette->findBestfit(c, c, c, a, new_mask_color, fitCriteria);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: narrowing conversion from 'color_t' (aka 'unsigned int') to signed type 'int' is implementation-defined [bugprone-narrowing-conversions]

              *dst_it = palette->findBestfit(c, c, c, a, new_mask_color, fitCriteria);
                                                   ^

else
*dst_it = palette->findBestfit(c, c, c, a, new_mask_color);
*dst_it = palette->findBestfit(c, c, c, a, new_mask_color, fitCriteria);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: narrowing conversion from 'color_t' (aka 'unsigned int') to signed type 'int' is implementation-defined [bugprone-narrowing-conversions]

              *dst_it = palette->findBestfit(c, c, c, a, new_mask_color, fitCriteria);
                                                         ^

else
*dst_it = palette->findBestfit(r, g, b, a, new_mask_color);
*dst_it = palette->findBestfit(r, g, b, a, new_mask_color, fitCriteria);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: narrowing conversion from 'color_t' (aka 'unsigned int') to signed type 'int' is implementation-defined [bugprone-narrowing-conversions]

                *dst_it = palette->findBestfit(r, g, b, a, new_mask_color, fitCriteria);
                                                           ^

@@ -68,7 +68,8 @@ namespace render {
bool is_background,
doc::color_t new_mask_color,
doc::rgba_to_graya_func toGray = nullptr,
TaskDelegate* delegate = nullptr);
TaskDelegate* delegate = nullptr,
const doc::FitCriteria fitCriteria = doc::FitCriteria::DEFAULT);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: parameter 'fitCriteria' is const-qualified in the function declaration; const-qualification of parameters only has an effect in function definitions [readability-avoid-const-params-in-decls]

Suggested change
const doc::FitCriteria fitCriteria = doc::FitCriteria::DEFAULT);
doc::FitCriteria fitCriteria = doc::FitCriteria::DEFAULT);

Copy link
Collaborator

@aseprite-bot aseprite-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clang-tidy made some suggestions

src/doc/sprite.h Show resolved Hide resolved
@@ -160,7 +161,8 @@
const RgbMapFor forLayer) const;
RgbMap* rgbMap(const frame_t frame,
const RgbMapFor forLayer,
RgbMapAlgorithm mapAlgo) const;
const RgbMapAlgorithm mapAlgo,
const FitCriteria fitCriteria = FitCriteria::DEFAULT) const;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: parameter 'fitCriteria' is const-qualified in the function declaration; const-qualification of parameters only has an effect in function definitions [readability-avoid-const-params-in-decls]

Suggested change
const FitCriteria fitCriteria = FitCriteria::DEFAULT) const;
FitCriteria fitCriteria = FitCriteria::DEFAULT) const;

Copy link
Collaborator

@aseprite-bot aseprite-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clang-tidy made some suggestions

@@ -564,6 +592,23 @@ void ChangePixelFormatCommand::onLoadParams(const Params& params)
m_rgbmap = Preferences::instance().quantization.rgbmapAlgorithm();
}

// TODO change this with NewParams as in ColorQuantizationParams
std::string fitCriteria = params.get("fitCriteria");
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: variable 'fitCriteria' of type 'std::string' (aka 'basic_string') can be declared 'const' [misc-const-correctness]

Suggested change
std::string fitCriteria = params.get("fitCriteria");
std::string const fitCriteria = params.get("fitCriteria");

src/doc/sprite.h Outdated
@@ -258,6 +260,9 @@ namespace doc {
mutable RgbMapAlgorithm m_rgbMapAlgorithm;
mutable std::unique_ptr<RgbMap> m_rgbMap;

// Last 'color fit criteria' used on indexed conversions
mutable FitCriteria m_fitCriteria = FitCriteria::DEFAULT;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we really need this in doc::Sprite?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right! We don't need it, and we don't need mutable RgbMapAlgorithm m_rgbMapAlgorithm; either. I'll add this parameters into class Rgbmap.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually moving it to the RgbMap is a similar case. If you think it makes more sense to move them inside RgbMap go for it (not sure if this FitCriteria is only used inside the RgbMap and not in the findBest of the palette).

One important thing I'm missing and probably I thought incorrectly was that this FitCriteria was something used in the Sprite > Color Mode > More Options > Advance only. But it looks like you are planning as part of the whole program configuration. Something particular for the Octree (if I'm guided by the #2787 title), or something that can be used in both rgbmaps (rgb5a3/octree). What is the plan with this PR?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In a first approximation, I thought of it only for octree quantization. But I tested it with RGB5A3 and it can be processed without problems. So I extended it for both rgbmaps.
I added a global setting to set the default value in Advanced conversion RGB->Indexed, then added this setting to work with Sprite > ColorMode > Indexed too.

We can stop here and wait for the artist's reception to see if it is feasible to extend this option further (include parameter in lua for example).

All of these other 'color fit criteria' in the Palette::findBestFit function can be useful for artists who want to fit any RGB image to a custom palette and achieve a more appropriate color selection.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dacap I will limit the feature to be used only in Sprite > Color Mode > More Options > Advance

Copy link
Collaborator

@aseprite-bot aseprite-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clang-tidy made some suggestions

virtual RgbMapAlgorithm rgbamapAlgorithm() const = 0;

// Color Best Fit Criteria used to generate the rgbmap
virtual FitCriteria fitCriteria() const = 0;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: unknown type name 'FitCriteria' [clang-diagnostic-error]

    virtual FitCriteria fitCriteria() const = 0;
            ^


// Color Best Fit Criteria used to generate the rgbmap
virtual FitCriteria fitCriteria() const = 0;
virtual void fitCriteria(const FitCriteria fitCriteria) = 0;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: parameter 'fitCriteria' is const-qualified in the function declaration; const-qualification of parameters only has an effect in function definitions [readability-avoid-const-params-in-decls]

Suggested change
virtual void fitCriteria(const FitCriteria fitCriteria) = 0;
virtual void fitCriteria(FitCriteria fitCriteria) = 0;


// Color Best Fit Criteria used to generate the rgbmap
virtual FitCriteria fitCriteria() const = 0;
virtual void fitCriteria(const FitCriteria fitCriteria) = 0;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: unknown type name 'FitCriteria' [clang-diagnostic-error]

    virtual void fitCriteria(const FitCriteria fitCriteria) = 0;
                                   ^

@Gasparoken
Copy link
Member Author

This new parameter called 'color fit criteria' is used to compare colors under different criteria in the Palette::findBestFit function.
A possible alternative to emplace the 'color fit criteria' conversion process is within the Rgbmap class.
I'll try to refactor this to include the conversion function inside Rgbmap only.

Copy link
Collaborator

@aseprite-bot aseprite-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clang-tidy made some suggestions

There were too many comments to post at once. Showing the first 25 out of 38. Check the log or trigger a new build to see more.

@@ -35,19 +35,21 @@ void OctreeNode::addColor(color_t c, int level, OctreeNode* parent,
(*m_children)[index].addColor(c, level + 1, this, paletteIndex, levelDeep);
}

int OctreeNode::mapColor(int r, int g, int b, int a, int mask_index, const Palette* palette, int level) const
int OctreeNode::mapColor(int r, int g, int b, int a, int mask_index,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: function 'mapColor' is within a recursive call chain [misc-no-recursion]

int OctreeNode::mapColor(int  r, int g, int b, int a, int mask_index,
                ^
Additional context

src/doc/octree_map.cpp:37: example recursive call chain, starting from function 'mapColor'

int OctreeNode::mapColor(int  r, int g, int b, int a, int mask_index,
                ^

src/doc/octree_map.cpp:51: Frame #1: function 'mapColor' calls function 'mapColor' here:

  return (*m_children)[index].mapColor(r, g, b, a, mask_index, palette, level + 1, octree);
         ^

src/doc/octree_map.cpp:51: ... which was the starting point of the recursive call chain; there may be other cycles

  return (*m_children)[index].mapColor(r, g, b, a, mask_index, palette, level + 1, octree);
         ^

@@ -135,9 +141,22 @@ class OctreeMap : public RgbMap {
const int levelDeep = 7);

// RgbMap impl
void regenerateMap(const Palette* palette, const int maskIndex) override;
void regenerateMap(const Palette* palette,
const int maskIndex,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: parameter 'maskIndex' is const-qualified in the function declaration; const-qualification of parameters only has an effect in function definitions [readability-avoid-const-params-in-decls]

Suggested change
const int maskIndex,
int maskIndex,

void regenerateMap(const Palette* palette, const int maskIndex) override;
void regenerateMap(const Palette* palette,
const int maskIndex,
const FitCriteria fitCriteria) override;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: parameter 'fitCriteria' is const-qualified in the function declaration; const-qualification of parameters only has an effect in function definitions [readability-avoid-const-params-in-decls]

Suggested change
const FitCriteria fitCriteria) override;
FitCriteria fitCriteria) override;

int size = std::min(256, int(m_colors.size()));
int bestfit = 0;
int lowest = std::numeric_limits<int>::max();
int size = std::min(256, int(m_colors.size()));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: variable 'size' of type 'int' can be declared 'const' [misc-const-correctness]

Suggested change
int size = std::min(256, int(m_colors.size()));
int const size = std::min(256, int(m_colors.size()));

for (int i=0; i<size; ++i) {
color_t rgb = m_colors[i];
for (int i=0; i<size; ++i) {
color_t rgb = m_colors[i];
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: variable 'rgb' of type 'color_t' (aka 'unsigned int') can be declared 'const' [misc-const-correctness]

Suggested change
color_t rgb = m_colors[i];
color_t const rgb = m_colors[i];

int RgbMapBase::findBestfit(int r, int g, int b, int a,
int mask_index) const
{
ASSERT(r >= 0 && r <= 255);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: boolean expression can be simplified by DeMorgan's theorem [readability-simplify-boolean-expr]

  ASSERT(r >= 0 && r <= 255);
  ^
Additional context

laf/base/debug.h:60: expanded from macro 'ASSERT'

    if (!(condition)) {                                                   \
        ^

int mask_index) const
{
ASSERT(r >= 0 && r <= 255);
ASSERT(g >= 0 && g <= 255);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: boolean expression can be simplified by DeMorgan's theorem [readability-simplify-boolean-expr]

  ASSERT(g >= 0 && g <= 255);
  ^
Additional context

laf/base/debug.h:60: expanded from macro 'ASSERT'

    if (!(condition)) {                                                   \
        ^

{
ASSERT(r >= 0 && r <= 255);
ASSERT(g >= 0 && g <= 255);
ASSERT(b >= 0 && b <= 255);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: boolean expression can be simplified by DeMorgan's theorem [readability-simplify-boolean-expr]

  ASSERT(b >= 0 && b <= 255);
  ^
Additional context

laf/base/debug.h:60: expanded from macro 'ASSERT'

    if (!(condition)) {                                                   \
        ^

ASSERT(r >= 0 && r <= 255);
ASSERT(g >= 0 && g <= 255);
ASSERT(b >= 0 && b <= 255);
ASSERT(a >= 0 && a <= 255);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: boolean expression can be simplified by DeMorgan's theorem [readability-simplify-boolean-expr]

  ASSERT(a >= 0 && a <= 255);
  ^
Additional context

laf/base/debug.h:60: expanded from macro 'ASSERT'

    if (!(condition)) {                                                   \
        ^


int bestfit = 0;
double lowest = std::numeric_limits<double>::max();
int size = m_palette->size();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: variable 'size' of type 'int' can be declared 'const' [misc-const-correctness]

Suggested change
int size = m_palette->size();
int const size = m_palette->size();

Copy link
Collaborator

@aseprite-bot aseprite-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clang-tidy made some suggestions

rgbToOtherSpace(x, y, z);

for (int i=0; i<size; ++i) {
color_t rgb = m_palette->getEntry(i);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: variable 'rgb' of type 'color_t' (aka 'unsigned int') can be declared 'const' [misc-const-correctness]

Suggested change
color_t rgb = m_palette->getEntry(i);
color_t const rgb = m_palette->getEntry(i);

double Zpal = double(rgba_getb(rgb));
// Palette color conversion RGB-->XYZ and r,g,b is assumed CIE XYZ
rgbToOtherSpace(Xpal, Ypal, Zpal);
double xDiff = x - Xpal;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: variable 'xDiff' of type 'double' can be declared 'const' [misc-const-correctness]

Suggested change
double xDiff = x - Xpal;
double const xDiff = x - Xpal;

// Palette color conversion RGB-->XYZ and r,g,b is assumed CIE XYZ
rgbToOtherSpace(Xpal, Ypal, Zpal);
double xDiff = x - Xpal;
double yDiff = y - Ypal;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: variable 'yDiff' of type 'double' can be declared 'const' [misc-const-correctness]

Suggested change
double yDiff = y - Ypal;
double const yDiff = y - Ypal;

rgbToOtherSpace(Xpal, Ypal, Zpal);
double xDiff = x - Xpal;
double yDiff = y - Ypal;
double zDiff = z - Zpal;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: variable 'zDiff' of type 'double' can be declared 'const' [misc-const-correctness]

Suggested change
double zDiff = z - Zpal;
double const zDiff = z - Zpal;

double xDiff = x - Xpal;
double yDiff = y - Ypal;
double zDiff = z - Zpal;
double aDiff = double(a - rgba_geta(rgb)) / 128.0;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: variable 'aDiff' of type 'double' can be declared 'const' [misc-const-correctness]

Suggested change
double aDiff = double(a - rgba_geta(rgb)) / 128.0;
double const aDiff = double(a - rgba_geta(rgb)) / 128.0;

protected:
FitCriteria m_fitCriteria;
const Palette* m_palette = nullptr;
int m_modifications = 0;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: member variable 'm_modifications' has protected visibility [misc-non-private-member-variables-in-classes]

  int m_modifications = 0;
      ^

FitCriteria m_fitCriteria;
const Palette* m_palette = nullptr;
int m_modifications = 0;
int m_maskIndex = 0;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: member variable 'm_maskIndex' has protected visibility [misc-non-private-member-variables-in-classes]

  int m_maskIndex = 0;
      ^

, m_maskIndex(0)
{
}
RgbMapRGB5A3::RgbMapRGB5A3() : m_map(MAPSIZE) {}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: performing an implicit widening conversion to type 'size_type' (aka 'unsigned long') of a multiplication performed in type 'int' [bugprone-implicit-widening-of-multiplication-result]

RgbMapRGB5A3::RgbMapRGB5A3() : m_map(MAPSIZE) {}
                                     ^
Additional context

src/doc/rgbmap_rgb5a3.cpp:22: expanded from macro 'MAPSIZE'

#define MAPSIZE (RSIZE*GSIZE*BSIZE*ASIZE)
                 ^

src/doc/rgbmap_rgb5a3.cpp:18: expanded from macro 'RSIZE'

#define RSIZE   32
                ^

src/doc/rgbmap_rgb5a3.cpp:24: make conversion explicit to silence this warning

RgbMapRGB5A3::RgbMapRGB5A3() : m_map(MAPSIZE) {}
                                     ^

src/doc/rgbmap_rgb5a3.cpp:22: expanded from macro 'MAPSIZE'

#define MAPSIZE (RSIZE*GSIZE*BSIZE*ASIZE)
                 ^

src/doc/rgbmap_rgb5a3.cpp:18: expanded from macro 'RSIZE'

#define RSIZE   32
                ^

src/doc/rgbmap_rgb5a3.cpp:24: perform multiplication in a wider type

RgbMapRGB5A3::RgbMapRGB5A3() : m_map(MAPSIZE) {}
                                     ^

src/doc/rgbmap_rgb5a3.cpp:22: expanded from macro 'MAPSIZE'

#define MAPSIZE (RSIZE*GSIZE*BSIZE*ASIZE)
                 ^

src/doc/rgbmap_rgb5a3.cpp:18: expanded from macro 'RSIZE'

#define RSIZE   32
                ^

// Bit activated on m_map entries that aren't yet calculated.
const int INVALID = 256;

public:
RgbMapRGB5A3();

// RgbMap impl
void regenerateMap(const Palette* palette, int maskIndex) override;
void regenerateMap(const Palette* palette,
const int maskIndex,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: parameter 'maskIndex' is const-qualified in the function declaration; const-qualification of parameters only has an effect in function definitions [readability-avoid-const-params-in-decls]

Suggested change
const int maskIndex,
int maskIndex,

void regenerateMap(const Palette* palette, int maskIndex) override;
void regenerateMap(const Palette* palette,
const int maskIndex,
const FitCriteria fitCriteria) override;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: parameter 'fitCriteria' is const-qualified in the function declaration; const-qualification of parameters only has an effect in function definitions [readability-avoid-const-params-in-decls]

Suggested change
const FitCriteria fitCriteria) override;
FitCriteria fitCriteria) override;

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

Successfully merging this pull request may close these issues.

Improve the octree quantization method using better Euclidean color distance
3 participants