Comment 1 Felipe Heidrich 2009-07-15 11:07:51 EDT

In graphics we usually don't validate all parameters before each call for performance reasons.

Comment 2 John Crowley 2009-07-15 11:26:30 EDT

Agree with the issue of performance, especially within the new Image(...) code.

You might consider additional checks in the PaletteData constructor since that is probably not invoked anywhere nearly as often as the Image operations.

An alternative that I've sometimes used would be to provide an additional PaletteData constructor which includes full validation and an on/off flag.

Something along the lines of:

public PaletteData(int redMask, int greenMask, int blueMask, boolean bFullCheck){
  this(redMask, greenMask, blueMask);   // normal constructor
  if(bFullCheck){
    ... full validation of params
      ... throw IllegalArgumentException if any problem
  }
}

The developer can now get full checking if desired, and then either change the signature or set the boolean to false for production.

A slightly larger JAR, but almost zero production performance penalty ... and save a couple of days during development trying to figure out why new Image(...) is aborting.

Comment 3 Silenio Quarti 2009-08-21 17:08:42 EDT

What kind of checks are you looking for? In this case the red mask is not continguos, but there are could be other problems, like the red and green masks overlap, etc.

 We cannot assume that there are predefined mask sets (i.e (0xF800, 0x7E0, 0x1F), (0xFF, 0xFF00, 0xFF0000) and since the depth is not know by the palette, we cannot check for overflows.

Comment 4 John Crowley 2009-08-21 18:51:44 EDT

I'm afraid that my knowledge is too shallow here to offer a good list of checks.

In general I would just say that the goal would be that any palette construction where the sequence:

  PaletteData plt = new PaletteData(.....);
  ImageData imgData=new ImageData(200, 200, 16, plt);
  Image img = new Image(display, imgData);

causes an Exception to be thrown in the 'new Image(...)' line should be caught in the 'new PaletteData(...)' line.  And I realize that in the PaletteData constructor you will not have all the data necessary to prevent every error.

Following your suggestions, possibly check for non-contiguous masks, overlapping masks, a zero mask (could that be legal?), etc.  These might be valid exceptions for any PaletteData.  There are probably several others that someone familiar with the code could design.

If you want to explore a bit further you might define an additional constructor along the lines:

    PaletteData(int redMask, int greenMask, int blueMask, int depth)

which would specify that the programmer is planning to use this palette for a particular pixel depth and additional logical checks can be performed assuming that depth (e.g. if depth = 16 then any mask > 0xFFFF is invalid).  depth==0 would perform just the base-level checks as listed above.  Invoking the existing constructor skips all the additional checks to avoid any change to performance or the execution of existing code.

Does that make sense?

Comment 5 Eclipse Webmaster 2019-09-06 15:33:06 EDT

This bug hasn't had any activity in quite some time. Maybe the problem got resolved, was a duplicate of something else, or became less pressing for some reason - or maybe it's still relevant but just hasn't been looked at yet.

If you have further information on the current state of the bug, please add it. The information can be, for example, that the problem still occurs, that you still want the feature, that more information is needed, or that the bug is (for whatever reason) no longer relevant.