#19198: [wxMSW | ] Why is the limit of a wxPalette in wxPalette::GetRGB thuogh it accepts any arbitrary length in the construction!

[wxTrac]

#19198: [wxMSW | ] Why is the limit of a wxPalette in wxPalette::GetRGB thuogh it
accepts any arbitrary length in the construction!
------------------------------------------+-------------------------
Reporter: tomay3000 | Owner:
Type: defect | Status: new
Priority: normal | Milestone:
Component: wxMSW | Version: dev-latest
Keywords: wxPalette, palette, RGB, 256 | Blocked By:
Blocking: | Patch: 0
------------------------------------------+-------------------------
Hello,
I needed some sort of color array in my application, so I decided to use a
`wxPalette` (Why not, a predefined class to use), and after a while I have
discovered that it is limited to only 256 entries when calling
`wxPalette::GetRGB` function in both `wxMSW` and `wxX11` where it can be
initialized to any arbitrary length.

So, instead of the check:

{{{

if (index < 0 || index > 255)
return false;

}}}

it should be as in `wxOSX` :

{{{

if (index < 0 || index >= M_PALETTEDATA->m_count)
return false;

}}}

Note also, the generic version is missing min check (which is 0).
For the moment, I just fell down back to using `std::vector<wxColour>`
instead.

am I missing something here or it is a bug!?

TIA.

--
Ticket URL: <https://trac.wxwidgets.org/ticket/19198>

[wxTrac]

#19198: [wxMSW & wxX11] Why is the limit of a wxPalette in wxPalette::GetRGB thuogh

it accepts any arbitrary length in the construction!

------------------------+------------------------------------------

Reporter: tomay3000 | Owner:
Type: defect | Status: new
Priority: normal | Milestone:
Component: wxMSW | Version: dev-latest

Resolution: | Keywords: wxPalette, palette, RGB, 256

Blocked By: | Blocking:
Patch: 0 |

------------------------+------------------------------------------
Changes (by tomay3000):

* cc: tomay3000@… (added)


--
Ticket URL: <https://trac.wxwidgets.org/ticket/19198#comment:1>

[wxTrac]

#19198: [wxMSW & wxX11] Why the index limit of a wxPalette in wxPalette::GetRGB is
only 256 thuogh it accepts any arbitrary length in the construction!
------------------------+------------------------------------------

Reporter: tomay3000 | Owner:
Type: defect | Status: new
Priority: normal | Milestone:
Component: wxMSW | Version: dev-latest

Resolution: | Keywords: wxPalette, palette, RGB, 256

Blocked By: | Blocking:
Patch: 0 |

------------------------+------------------------------------------
Changes (by tomay3000):

* cc: tomay3000@… (removed)


--
Ticket URL: <https://trac.wxwidgets.org/ticket/19198#comment:2>

[rohit agarwal]

A palette is intended to be a map.
While all modern graphics subsystems have 8bits assigned for each of R,G,B
the 8-bit palette makes the representation of an image in memory compact.
16-bit color is represent as 5-6-5, there is no palette.
The palette selects an optimum map of 256 points in the 256x256x256 space of colors.
Hence, the way it is sized.
I’m guessing the mac version intended it to be 256 or lower, not higher.
I would not use a palette as a convenient way of representing a color array.
unsigned char colortable[n][3] would be a better choice

> --
> You received this message because you are subscribed to the Google Groups "wx-dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to wx-dev+un...@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/wx-dev/048.1b6f67aa6aa52d2f122999960b63c554%40wxwiki.org.

[wxTrac]

#19198: [wxMSW & wxX11] Why the index limit of a wxPalette in wxPalette::GetRGB is
only 256 thuogh it accepts any arbitrary length in the construction!
------------------------+------------------------------------------
Reporter: tomay3000 | Owner:
Type: defect | Status: closed

Priority: normal | Milestone:
Component: wxMSW | Version: dev-latest

Resolution: invalid | Keywords: wxPalette, palette, RGB, 256

Blocked By: | Blocking:
Patch: 0 |

------------------------+------------------------------------------
Changes (by vadz):

* status: new => closed
* resolution: => invalid


Comment:

Sorry, I don't understand what is this about, i.e. what exactly is the
bug. The code in wxMSW version of `wxPalette` looks weird (i.e. I don't
know what is this 255 doing there), but I have no idea how to reproduce
the problem if there is one and I'm not curious enough to spend time on
this class which is absolutely useless nowadays as nobody uses palette-
based video modes anyhow since many years.

If you want to store colours, using `std::vector` is perfectly fine and
definitely better than a platform-dependent (!) class for doing it.

Please reopen if there is a real problem here.

--
Ticket URL: <https://trac.wxwidgets.org/ticket/19198#comment:3>

[wxTrac]

#19198: [wxMSW & wxX11] Why the index limit of a wxPalette in wxPalette::GetRGB is
only 256 thuogh it accepts any arbitrary length in the construction!
------------------------+------------------------------------------
Reporter: tomay3000 | Owner:
Type: defect | Status: closed

Priority: normal | Milestone:
Component: wxMSW | Version: dev-latest

Resolution: invalid | Keywords: wxPalette, palette, RGB, 256

Blocked By: | Blocking:
Patch: 0 |

------------------------+------------------------------------------
Changes (by tomay3000):

* Attachment "palette-fix.patch" added.

Patch

[wxTrac]

#19198: [wxMSW & wxX11] Why the index limit of a wxPalette in wxPalette::GetRGB is
only 256 thuogh it accepts any arbitrary length in the construction!
------------------------+------------------------------------------
Reporter: tomay3000 | Owner:
Type: defect | Status: closed
Priority: lowest | Milestone:
Component: wxMSW | Version: dev-latest
Resolution: invalid | Keywords: wxPalette, palette, RGB, 256
Blocked By: | Blocking:
Patch: 1 |
------------------------+------------------------------------------
Changes (by tomay3000):

* priority: normal => lowest
* cc: tomay3000@… (added)
* patch: 0 => 1


Comment:

Replying to [comment:3 vadz]:

> Sorry, I don't understand what is this about, i.e. what exactly is the
bug. The code in wxMSW version of `wxPalette` looks weird (i.e. I don't

know what is this 255 doing there).

That is exactly the problem, 255 has nothing to do there. And I have
uploaded a fix patch for it.
----
> but I have no idea how to reproduce the problem if there is one.
{{{
const unsigned char *pR;
const unsigned char *pG;
const unsigned char *pB;
// Init and Fill pR, pG and pB.
...
// Create the palette.
wxPalette palette(1000, pR, pG, pB);

// Get an RGB colour of an index greater than 255.
// [this is the bug].
// wxPalette::GetRGB() will return false.
}}}
----

> this class which is absolutely useless nowadays as nobody uses palette-
based video modes anyhow since many years.

At least, my first thought before I later decide to use
`std::vector<wxColour>` instead was to use it, somebody else may in the
future.
----

> If you want to store colours, using `std::vector` is perfectly fine and
definitely better than a platform-dependent (!) class for doing it.

That's exactly what I did when I discovered the bug.

TIA.

--
Ticket URL: <https://trac.wxwidgets.org/ticket/19198#comment:4>

[wxTrac]

#19198: [wxMSW & wxX11] Why the index limit of a wxPalette in wxPalette::GetRGB is
only 256 thuogh it accepts any arbitrary length in the construction!
------------------------+------------------------------------------
Reporter: tomay3000 | Owner:
Type: defect | Status: closed
Priority: lowest | Milestone:
Component: wxMSW | Version: dev-latest
Resolution: invalid | Keywords: wxPalette, palette, RGB, 256
Blocked By: | Blocking:

Patch: 1 |
------------------------+------------------------------------------
Changes (by tomay3000):

* Attachment "palette-fix.patch" added.

Patch

[wxTrac]

#19198: [wxMSW & wxX11] Why the index limit of a wxPalette in wxPalette::GetRGB is
only 256 thuogh it accepts any arbitrary length in the construction!
------------------------+------------------------------------------
Reporter: tomay3000 | Owner:
Type: defect | Status: closed
Priority: lowest | Milestone:
Component: wxMSW | Version: dev-latest
Resolution: invalid | Keywords: wxPalette, palette, RGB, 256
Blocked By: | Blocking:

Patch: 1 |
------------------------+------------------------------------------
Changes (by tomay3000):

* Attachment "palette-fix.patch" added.

Patch

[wxTrac]

#19198: [wxMSW & wxX11] The index of a wxPalette in wxPalette::GetRGB() has a limit
up to only 255, though it accepts any arbitrary length in the construction
------------------------+------------------------------------------
Reporter: tomay3000 | Owner:
Type: defect | Status: reopened
Priority: lowest | Milestone:
Component: wxMSW | Version: dev-latest
Resolution: | Keywords: wxPalette, palette, RGB, 256
Blocked By: | Blocking:

Patch: 1 |
------------------------+------------------------------------------
Changes (by tomay3000):

* status: closed => reopened
* cc: tomay3000@… (removed)
* resolution: invalid =>


--
Ticket URL: <https://trac.wxwidgets.org/ticket/19198#comment:5>

[wxTrac]

#19198: [wxMSW & wxX11] The index of a wxPalette in wxPalette::GetRGB() has a limit

up to only 255, though it accepts any arbitrary length in the construction
------------------------+------------------------------------------
Reporter: tomay3000 | Owner:
Type: defect | Status: reopened
Priority: lowest | Milestone:
Component: wxMSW | Version: dev-latest
Resolution: | Keywords: wxPalette, palette, RGB, 256
Blocked By: | Blocking:
Patch: 1 |
------------------------+------------------------------------------

Comment (by vadz):

Thanks, will merge soon.

--
Ticket URL: <https://trac.wxwidgets.org/ticket/19198#comment:6>

[wxTrac]

#19198: [wxMSW & wxX11] The index of a wxPalette in wxPalette::GetRGB() has a limit
up to only 255, though it accepts any arbitrary length in the construction
------------------------+------------------------------------------

Reporter: tomay3000 | Owner: Vadim Zeitlin <vadim@…>
Type: defect | Status: closed
Priority: lowest | Milestone:
Component: wxMSW | Version: dev-latest
Resolution: fixed | Keywords: wxPalette, palette, RGB, 256
Blocked By: | Blocking:
Patch: 1 |
------------------------+------------------------------------------
Changes (by Vadim Zeitlin <vadim@…>):

* owner: => Vadim Zeitlin <vadim@…>
* status: reopened => closed
* resolution: => fixed


Comment:

In [changeset:"b52f00492e4c6ff199488e1997fff19a8e158bb7/git-wxWidgets"
b52f00492/git-wxWidgets]:
{{{
#!CommitTicketReference repository="git-wxWidgets"
revision="b52f00492e4c6ff199488e1997fff19a8e158bb7"
Fix checks for wxPalette index validity

Consistently check that the index is valid in all ports, instead of
using hard-coded 255 rather than the actual number of colours in some of
them and forgetting to check that the index is positive in others.

Closes #19198.
}}}

--
Ticket URL: <https://trac.wxwidgets.org/ticket/19198#comment:7>