nanogui: Thread: GrDrawImageToFit fail with PNG image with alpha layer

This:

> GrDrawImageToFit(GrNewPixmap(640, 480, NULL),
>                  myGC, 0, 0, 640, 480,
>                  GrLoadImageFromFile("/path/to/png_with_alpha_channel.png", 0));

fails with the following error:

> nano-X: /path/to/microwin/src/drivers/fblin24.c: 35: linear24_drawpixel: Assertion `c < psd->ncolors' failed.

The function is defined like this:

> /* Set pixel at x, y, to pixelval c*/
> static void linear24_drawpixel(PSD psd, MWCOORD x, MWCOORD y, MWPIXELVAL c)

and the assertion looks like:

>    assert (c < psd->ncolors);

It seems like the PNG buffer is read as 32 bit values, but the
function quickly thereafter turn the pixelval into r, g and b:

>	r = PIXEL888RED(c);
>	g = PIXEL888GREEN(c);
>	b = PIXEL888BLUE(c);

And the pure 'c' is never used again. How about just remove that
assertion? What would then fail? What would else be a way to get
fblin24 handle png's with alpha chan?

Best Regards
Daniel

On Thu, Jun 03, 2010 at 03:00:57PM +0200, Daniel Nyström wrote:
> This:
> 
> > GrDrawImageToFit(GrNewPixmap(640, 480, NULL),
> >                  myGC, 0, 0, 640, 480,
> >                  GrLoadImageFromFile("/path/to/png_with_alpha_channel.png", 0));
> 
> fails with the following error:
> 
> > nano-X: /path/to/microwin/src/drivers/fblin24.c: 35: linear24_drawpixel: Assertion `c < psd->ncolors' failed.
> 
> The function is defined like this:
> 
> > /* Set pixel at x, y, to pixelval c*/
> > static void linear24_drawpixel(PSD psd, MWCOORD x, MWCOORD y, MWPIXELVAL c)
> 
> and the assertion looks like:
> 
> >    assert (c < psd->ncolors);
> 
> It seems like the PNG buffer is read as 32 bit values, but the
> function quickly thereafter turn the pixelval into r, g and b:
> 
> >	r = PIXEL888RED(c);
> >	g = PIXEL888GREEN(c);
> >	b = PIXEL888BLUE(c);
> 
> And the pure 'c' is never used again. How about just remove that
> assertion? What would then fail? What would else be a way to get
> fblin24 handle png's with alpha chan?

the assertion is incorrect for non-palettized video modes.  I would just
remove the asserts, or change it to

	assert(psd->pixtype != MWPF_PALETTE || c < psd->ncolors)

-- 
  Aaron J. Grier | "Not your ordinary poofy goof." | ####@####.####

2010/6/3 Aaron J. Grier ####@####.####
> On Thu, Jun 03, 2010 at 03:00:57PM +0200, Daniel Nyström wrote:
>> >    assert (c < psd->ncolors);
>
> the assertion is incorrect for non-palettized video modes.  I would just
> remove the asserts, or change it to
>
>        assert(psd->pixtype != MWPF_PALETTE || c < psd->ncolors)

Since fblin24.c is specifically for 24bit RGB I guess pixtype will
never be anything but MWPF_TRUECOLOR888, so just removing the assert
would do?

On Thu, Jun 03, 2010 at 08:13:32PM +0200, Daniel Nyström wrote:
> 2010/6/3 Aaron J. Grier ####@####.####
> > On Thu, Jun 03, 2010 at 03:00:57PM +0200, Daniel Nyström wrote:
> >> >    assert (c < psd->ncolors);
> >
> > the assertion is incorrect for non-palettized video modes.  I would just
> > remove the asserts, or change it to
> >
> >        assert(psd->pixtype != MWPF_PALETTE || c < psd->ncolors)
> 
> Since fblin24.c is specifically for 24bit RGB I guess pixtype will
> never be anything but MWPF_TRUECOLOR888, so just removing the assert
> would do?

that's what I figured and why I simply removed the assert.  as to what
should be commit back to the trunk, I defer to Greg's judgement.

-- 
  Aaron J. Grier | "Not your ordinary poofy goof." | ####@####.####

: > static void linear24_drawpixel(PSD psd, MWCOORD x, MWCOORD y, MWPIXELVAL 
c)
:
: and the assertion looks like:
:
: >    assert (c < psd->ncolors);
:
: It seems like the PNG buffer is read as 32 bit values, but the

What version are you running?

The git repository should not have this issue.  I am in the
process of moving to alpha-compositing support, so the
assert fails because the high byte of the RGB color value
defaults to 255 alpha, which is outside the 24-bit color
range.

Regards,

Greg 

: the assertion is incorrect for non-palettized video modes.  I would just
: remove the asserts, or change it to
: 
: assert(psd->pixtype != MWPF_PALETTE || c < psd->ncolors)

The above is incorrect, as the assert is in the fblin24 driver,
which is only used for non-palette 24 bit framebuffers.

I have recently completed some very extensive driver updates.
I think all of this is fixed in the git repository, but am awaiting
Daniel's statement as to which version he's running.

Regards,

Greg

> Since fblin24.c is specifically for 24bit RGB I guess pixtype will
never be anything but MWPF_TRUECOLOR888, so just removing the assert
would do?

Yes - removing the assert is correct.  

Regards,

Greg

2010/6/4 Greg Haerr ####@####.####
> : > static void linear24_drawpixel(PSD psd, MWCOORD x, MWCOORD y, MWPIXELVAL
> c)
> :
> : and the assertion looks like:
> :
> : >    assert (c < psd->ncolors);
> :
> : It seems like the PNG buffer is read as 32 bit values, but the
>
> What version are you running?

I'm currently using a git clone of commit
265f32ae12c9e0310f4978b951060e5b77d91231.

2010/6/7 Greg Haerr ####@####.####
>> I'm currently using a git clone of commit
>
> Thanks Daniel - I somehow forgot these asserts in
> the fblin24 driver and have removed them.
>
> Are you running this same driver/hardware for your
> earlier anti-aliasing font problem?  Is it fixed?

I'm using this same driver, and I actually haven't noticed any font
rendering artifacts in a while. Maybe I should try out the test
application once again, but I'm currently very overloaded at work.
Trying out Freetype 2.3.9 along with the new PCF rendering is also on
my todo-list.

Regards
Daniel