nanogui: Thread: patch for microwindows in wdtv live

Hi Greg:

WDTV live is a HD media player, which is mass production.
http://www.wdc.com/en/products/Products.asp?DriveID=735 

It releases GPL source code which includes microwindows 0.90
GPL:
http://support.wdc.com/product/download.asp?groupid=1003&sid=120&lang=en 
WDTV_Live_GPL_Code_package.1.01.24.zip  (ZIP  219 MB)

1. devdraw better performance 
in include\device.h , it defines several macros:
MACRO_DRAWPIXEL, BPP32_DRAWPIXEL  etc.
in engine\devdraw.c, GdDrawImage, it has performance issue.
in git source code:
===================
} else if (bpp > 8) { /* 16, 18, 24, or 32bpp*/ 
  while (height > 0) { 
===================
It will check the switch condition in this "while (height > 0)"  loop frequently, which has performance issue.

Maybe you could merge wdtv code to git, this is a critical issue.

2. JPEG EOI 
in engine\devimage.c, LoadJPEG , it checks the EOI (end of image) of jpeg, which will make system hang in jpeg lib for certain platform, (which is happened in network image transport), but in x86 it will not hang.

Maybe you could merge this wdtv code to git, but this is not a critical issue.

3. png alpha blending
in engine\devimage.c, it adds 
#if defined(ALPHA_BLENDING_FOR_PNG) 
and it will check "pnginfo->num_trans > 0" which is used for PNG_EXPAND_tRNS.
===================
/* set load image pixel format 'BGRA' */
if ((colourtype == PNG_COLOR_TYPE_RGB && pnginfo->num_trans > 0)  ||
    colourtype == PNG_COLOR_TYPE_RGB_ALPHA)
png_set_bgr(state);
=================== 
Support 24bits RGB and tRNS type png in load_png function. which will handle it as 32bits and set a bgr for drawing (which will do a bgr to rgb swap for 32 bits again)

BTW:
Glad to see the web site is updated in 2010. 

> WDTV live is a HD media player, which is mass production.

Actually, I am familiar with the player.  I didn't
know it's running microwindows!  Very nice.  I recently
evaluated the device.

Are you involved with the programming team?  If so,
I would like to have an off-list discussion with
you regarding my evaluation and comments of this
very interesting device.  Also, are you trying/planning
on moving to the (v0.92) git version of microwindows?


> WDTV_Live_GPL_Code_package.1.01.24.zip  (ZIP  219 MB)

I have downloaded and looked at the source.
My comments follow.  It would be interesting
to know all of the areas you have changed.


I will diff with v0.90, are there other areas
than below that need changing in order to
move to the v0.92 git version?



> 1. devdraw better performance
in include\device.h , it defines several macros:
MACRO_DRAWPIXEL, BPP32_DRAWPIXEL  etc.
in engine\devdraw.c, GdDrawImage, it has performance issue.

I have looked at this and agree, I will add
the optimization into the git source tree.



> 2. JPEG EOI
in engine\devimage.c, LoadJPEG , it checks the EOI (end of image) of jpeg, 
which will make system hang in jpeg lib for certain platform, (which is 
happened in network image transport), but in x86 it will not hang.

The newer v0.92 handles incoming data streams a bit
differently.  I would like to know more information
to correctly handle this modification.  Is the
problem that the jpeg library starts decoding before
the whole file is received?  I think I can add
code that checks that last two characters of the
buffer before allowing the image to be decoded.
Some changes will have to be made to the current
buffer mechanism to handle seek to end of file, but
not many.




> 3. png alpha blending
in engine\devimage.c, it adds
#if defined(ALPHA_BLENDING_FOR_PNG)
and it will check "pnginfo->num_trans > 0" which is used for 
PNG_EXPAND_tRNS.
===================
/* set load image pixel format 'BGRA' */
if ((colourtype == PNG_COLOR_TYPE_RGB && pnginfo->num_trans > 0)  ||
    colourtype == PNG_COLOR_TYPE_RGB_ALPHA)
png_set_bgr(state);

This modification looks like no problem.  I will likely
leave out the ALPHA_BLENDING_FOR_PNG define, and
set always on, since the v0.92 always supports alpha
channel blending in images.  Thus, if the png has
alpha channel, it will always work.

> Glad to see the web site is updated in 2010.

Yes, it has been too long!  I am very close to releasing
a 0.92 from the git repository.  There have been
several important patches that have come in lately,
and yours is the latest.  I will notify you when
these changes are incorporated, and if you could
let me know whether v0.92 could be used for production
WDTV use, I would appreciate it!

Regards,

Greg

Hi,

I have looked much more extensively at all the
changes that are in the WDTV source tree, based
on a release after 0.90, but before 0.91.

I see that you have been quite busy, and there
are still quite a few items that have been added,
which will need to be moved into 0.92 git in order
to use the current codebase.  These items include:

o engine/devmalloc.c STATIC memory allocations
o static buffers for images
o NO_CHARMAP_WORKAROUND fix in font_freetype{2}.c
o drivers/scr_em86xx.c and scr_smp863x.c screen drivers
o MWROP_COPY_RK and MWROP_COPY_IK handling in 16 and 32bpp drivers
o TRANSLATE_ESC handling in kbd_tty.c
o brightness handling (requiring float) in devopen.c
o chinese font and HBF handling in freetype 2
o fast draw enhancements in devdraw.c::GdDrawImage
o jpeg setjmp and EOI handling
o png alpha channel and BGR handling


I am very interested in helping move all required
changes into the current pre-0.92 source tree,
but have some questions, which I've outlined
below.  I would like to know whether you're 
already trying to move to 0.92, given it
doesn't contain any of the features above (yet!).

Here are my comments on each of the above, in 
more detail.

o engine/devmalloc.c STATIC memory allocations
o static buffers for images

These can be fairly easily handled by using
a specific memory allocator in all the image
routines, which could be redirected to your
STATIC_malloc() routines in devmalloc.c.


o NO_CHARMAP_WORKAROUND fix in font_freetype{2}.c

I'm not quite certain what this is for, it appears
to handle character values > 0x80 differently.  Can
you comment?  Also, are you currently using 
FT2, and which version?  There have been many
changes adopted in FT2 since then, and the cache
code is now working in the repository.

o drivers/scr_em86xx.c and scr_smp863x.c screen drivers

Are these drivers used in WDTV, or are you using
the fblin32.c and fblin16.c drivers?

o MWROP_COPY_RK and MWROP_COPY_IK handling in 16 and 32bpp drivers

These should be easily added to the fblin16 and fblin32 drivers.

o TRANSLATE_ESC handling in kbd_tty.c

Easily added

o brightness handling (requiring float) in devopen.c

The current implementation drags in floating point.
It would be nice to have a brightness control
capability that didn't require float.

o chinese font and HBF handling in freetype 2

Not sure if this is still required.  Are these
mods, including the hbf.c files, still required
when using FT2 fonts?  Are you using FT2 fonts
on the device, or only hbf.c and compiled-in
arial fonts?

o fast draw enhancements in devdraw.c::GdDrawImage

I like this modification.  However, in looking more
deeply, the speedup assumes a framebuffer display
and writes directly, bypassing the driver.  This
won't work using non-fb display drivers.  Thus, 
I will need to apply a bit more work so that 
your original speed improvement is held, and
the code works on any system (like X11 for test,
for instance). 

We might add a flag, PSF_HAVEFB, to indicate whether
the GdDrawImage routine can draw directly or must
go through the screen driver for this operation,
for instance.


o jpeg setjmp and EOI handling

I see there is also setjmp handling that is 
new, for what reason?  Is the reason for the EOI
handling that some jpegs are never received in
full, and that hangs the jpeg decoder?

o png alpha channel and BGR handling

There is currently alpha channel support in
0.92RC2 that should work.  However, there
isn't support for conversion to BGR format.
Is this to guarantee quick drawing the 
GdDrawImage routine, or for another reason?
I would like a sample .png for testing.

I may have left out other requirements, please
let me know.

Regards,

Greg

see comments below.


2010-02-22 



yut616 



发件人： Greg Haerr 
发送时间： 2010-02-22  09:57:47 
收件人： yut616; nanogui 
抄送： 
主题： Re: [nanogui] patch for microwindows in wdtv live 
 
Hi,
I have looked much more extensively at all the
changes that are in the WDTV source tree, based
on a release after 0.90, but before 0.91.
I see that you have been quite busy, and there
are still quite a few items that have been added,
which will need to be moved into 0.92 git in order
to use the current codebase.  These items include:
o engine/devmalloc.c STATIC memory allocations
o static buffers for images
o NO_CHARMAP_WORKAROUND fix in font_freetype{2}.c
o drivers/scr_em86xx.c and scr_smp863x.c screen drivers
o MWROP_COPY_RK and MWROP_COPY_IK handling in 16 and 32bpp drivers
o TRANSLATE_ESC handling in kbd_tty.c
o brightness handling (requiring float) in devopen.c
o chinese font and HBF handling in freetype 2
o fast draw enhancements in devdraw.c::GdDrawImage
o jpeg setjmp and EOI handling
o png alpha channel and BGR handling
I am very interested in helping move all required
changes into the current pre-0.92 source tree,
but have some questions, which I've outlined
below.  I would like to know whether you're 
already trying to move to 0.92, given it
doesn't contain any of the features above (yet!).
Here are my comments on each of the above, in 
more detail.
o engine/devmalloc.c STATIC memory allocations
o static buffers for images
These can be fairly easily handled by using
a specific memory allocator in all the image
routines, which could be redirected to your
STATIC_malloc() routines in devmalloc.c.

[TAO] 
This is used for avoid the memory fragment.

o NO_CHARMAP_WORKAROUND fix in font_freetype{2}.c
I'm not quite certain what this is for, it appears
to handle character values > 0x80 differently.  Can
you comment?  Also, are you currently using 
FT2, and which version?  There have been many
changes adopted in FT2 since then, and the cache
code is now working in the repository.

[TAO]
you could see HAVE_FREETYPE_2_SUPPORT=Y at the bottom of config file which will overwrite the old value.
This should be font provider related, it could be ignored.
Sometimes, it will make a little change in these files for special font.
This feature is not enabled in config.

o drivers/scr_em86xx.c and scr_smp863x.c screen drivers
Are these drivers used in WDTV, or are you using
the fblin32.c and fblin16.c drivers?

[TAO] this is the driver for sigma 863x chipset, gfx.

o MWROP_COPY_RK and MWROP_COPY_IK handling in 16 and 32bpp drivers
These should be easily added to the fblin16 and fblin32 drivers.


o TRANSLATE_ESC handling in kbd_tty.c
Easily added


o brightness handling (requiring float) in devopen.c
The current implementation drags in floating point.
It would be nice to have a brightness control
capability that didn't require float.

[TAO] please ignore it, the first line of "case MWPF_TRUECOLOR565" is using a return.

o chinese font and HBF handling in freetype 2
Not sure if this is still required.  Are these
mods, including the hbf.c files, still required
when using FT2 fonts?  Are you using FT2 fonts
on the device, or only hbf.c and compiled-in
arial fonts?

[TAO] HBF is a Hanzi Bitmap font.
USE_HBF_FONT in src/drivers/genfont.c (used for BIG5 font)
Maybe it is helpful in some low cost embedded devices with small size LCD.

o fast draw enhancements in devdraw.c::GdDrawImage
I like this modification.  However, in looking more
deeply, the speedup assumes a framebuffer display
and writes directly, bypassing the driver.  This
won't work using non-fb display drivers.  Thus, 
I will need to apply a bit more work so that 
your original speed improvement is held, and
the code works on any system (like X11 for test,
for instance). 
We might add a flag, PSF_HAVEFB, to indicate whether
the GdDrawImage routine can draw directly or must
go through the screen driver for this operation,
for instance.

[TAO] please note the BPP32_COLOR2PIXEL/BPP32_DRAWPIXEL, which has no switch section in it.

o jpeg setjmp and EOI handling
I see there is also setjmp handling that is 
new, for what reason?  Is the reason for the EOI
handling that some jpegs are never received in
full, and that hangs the jpeg decoder?

[TAO] When extract image from mp3 id3 tag will cause this issue if the image is uncompleted. 
setjmp could not handle the hang issue.

o png alpha channel and BGR handling
There is currently alpha channel support in
0.92RC2 that should work.  However, there
isn't support for conversion to BGR format.
Is this to guarantee quick drawing the 
GdDrawImage routine, or for another reason?
I would like a sample .png for testing.

[TAO] this png is 24bit, but with "tRNS Transparency" 
This is related to the libpng, just a special case for 24bit png (IHDR Bitdepth: 8 ). 
refer:
http://www.libpng.org/pub/png/spec/1.1/png-1.1.pdf
4.2.12 tRNS Transparency 

According to PNG_EXPAND_tRNS to build in an alpha channel using trans_value.
When the RGB is same as trans_value's RGB, it will set ALPHA to 0, otherwise it will set 0xFF.
======================
libpng-1.2.14/pngrtran.c
png_do_expand()
(row_info->color_type == PNG_COLOR_TYPE_RGB && trans_value) 
======================

BTW, png (with alpha channel) is much better for modern UI design than jpeg, PNG feature should be full supported.

I may have left out other requirements, please
let me know.
Regards,
Greg
---------------------------------------------------------------------
To unsubscribe, e-mail: ####@####.####
For additional commands, e-mail: ####@####.####

see comments below.


2010-02-22 



yut616 



发件人： Greg Haerr 
发送时间： 2010-02-22  09:57:47 
收件人： yut616; nanogui 
抄送： 
主题： Re: [nanogui] patch for microwindows in wdtv live 
Hi,
I have looked much more extensively at all the
changes that are in the WDTV source tree, based
on a release after 0.90, but before 0.91.
I see that you have been quite busy, and there
are still quite a few items that have been added,
which will need to be moved into 0.92 git in order
to use the current codebase.  These items include:
o engine/devmalloc.c STATIC memory allocations
o static buffers for images
o NO_CHARMAP_WORKAROUND fix in font_freetype{2}.c
o drivers/scr_em86xx.c and scr_smp863x.c screen drivers
o MWROP_COPY_RK and MWROP_COPY_IK handling in 16 and 32bpp drivers
o TRANSLATE_ESC handling in kbd_tty.c
o brightness handling (requiring float) in devopen.c
o chinese font and HBF handling in freetype 2
o fast draw enhancements in devdraw.c::GdDrawImage
o jpeg setjmp and EOI handling
o png alpha channel and BGR handling
I am very interested in helping move all required
changes into the current pre-0.92 source tree,
but have some questions, which I've outlined
below.  I would like to know whether you're 
already trying to move to 0.92, given it
doesn't contain any of the features above (yet!).
Here are my comments on each of the above, in 
more detail.
o engine/devmalloc.c STATIC memory allocations
o static buffers for images
These can be fairly easily handled by using
a specific memory allocator in all the image
routines, which could be redirected to your
STATIC_malloc() routines in devmalloc.c.

[TAO] 
This is used for avoid the memory fragment.

o NO_CHARMAP_WORKAROUND fix in font_freetype{2}.c
I'm not quite certain what this is for, it appears
to handle character values > 0x80 differently.  Can
you comment?  Also, are you currently using 
FT2, and which version?  There have been many
changes adopted in FT2 since then, and the cache
code is now working in the repository.

[TAO]
you could see HAVE_FREETYPE_2_SUPPORT=Y at the bottom of config file which will overwrite the old value.
This should be font provider related, it could be ignored.
Sometimes, it will make a little change in these files for special font.
This feature is not enabled in config.

o drivers/scr_em86xx.c and scr_smp863x.c screen drivers
Are these drivers used in WDTV, or are you using
the fblin32.c and fblin16.c drivers?

[TAO] this is the driver for sigma 863x chipset, gfx.

o MWROP_COPY_RK and MWROP_COPY_IK handling in 16 and 32bpp drivers
These should be easily added to the fblin16 and fblin32 drivers.


o TRANSLATE_ESC handling in kbd_tty.c
Easily added


o brightness handling (requiring float) in devopen.c
The current implementation drags in floating point.
It would be nice to have a brightness control
capability that didn't require float.

[TAO] please ignore it, the first line of "case MWPF_TRUECOLOR565" is using a return.

o chinese font and HBF handling in freetype 2
Not sure if this is still required.  Are these
mods, including the hbf.c files, still required
when using FT2 fonts?  Are you using FT2 fonts
on the device, or only hbf.c and compiled-in
arial fonts?

[TAO] HBF is a Hanzi Bitmap font.
USE_HBF_FONT in src/drivers/genfont.c (used for BIG5 font)
Maybe it is helpful in some low cost embedded devices with small size LCD.

o fast draw enhancements in devdraw.c::GdDrawImage
I like this modification.  However, in looking more
deeply, the speedup assumes a framebuffer display
and writes directly, bypassing the driver.  This
won't work using non-fb display drivers.  Thus, 
I will need to apply a bit more work so that 
your original speed improvement is held, and
the code works on any system (like X11 for test,
for instance). 
We might add a flag, PSF_HAVEFB, to indicate whether
the GdDrawImage routine can draw directly or must
go through the screen driver for this operation,
for instance.

[TAO] please note the BPP32_COLOR2PIXEL/BPP32_DRAWPIXEL, which has no switch section in it.

o jpeg setjmp and EOI handling
I see there is also setjmp handling that is 
new, for what reason?  Is the reason for the EOI
handling that some jpegs are never received in
full, and that hangs the jpeg decoder?

[TAO] When extract image from mp3 id3 tag will cause this issue if the image is uncompleted. 
setjmp could not handle the hang issue.

o png alpha channel and BGR handling
There is currently alpha channel support in
0.92RC2 that should work.  However, there
isn't support for conversion to BGR format.
Is this to guarantee quick drawing the 
GdDrawImage routine, or for another reason?
I would like a sample .png for testing.

[TAO] this png is 24bit, but with "tRNS Transparency" 
This is related to the libpng, just a special case for 24bit png (IHDR Bitdepth: 8 ). 
refer:
www(dot)libpng(dot)org/pub/png/spec/1.1/png-1.1.pdf (please change "dot" to ".")
4.2.12 tRNS Transparency 

According to PNG_EXPAND_tRNS to build in an alpha channel using trans_value.
When the RGB is same as trans_value's RGB, it will set ALPHA to 0, otherwise it will set 0xFF.
======================
libpng-1.2.14/pngrtran.c
png_do_expand()
(row_info->color_type == PNG_COLOR_TYPE_RGB && trans_value) 
======================

BTW, png (with alpha channel) is much better for modern UI design than jpeg, PNG feature should be full supported.

I may have left out other requirements, please
let me know.
Regards,
Greg
---------------------------------------------------------------------
To unsubscribe, e-mail: ####@####.####
For additional commands, e-mail: ####@####.####

see comments below.




发件人： Greg Haerr 
发送时间： 2010-02-22  09:57:47 
收件人： yut616; nanogui 
抄送： 
主题： Re: [nanogui] patch for microwindows in wdtv live 
Hi,
I have looked much more extensively at all the
changes that are in the WDTV source tree, based
on a release after 0.90, but before 0.91.
I see that you have been quite busy, and there
are still quite a few items that have been added,
which will need to be moved into 0.92 git in order
to use the current codebase.  These items include:
o engine/devmalloc.c STATIC memory allocations
o static buffers for images
o NO_CHARMAP_WORKAROUND fix in font_freetype{2}.c
o drivers/scr_em86xx.c and scr_smp863x.c screen drivers
o MWROP_COPY_RK and MWROP_COPY_IK handling in 16 and 32bpp drivers
o TRANSLATE_ESC handling in kbd_tty.c
o brightness handling (requiring float) in devopen.c
o chinese font and HBF handling in freetype 2
o fast draw enhancements in devdraw.c::GdDrawImage
o jpeg setjmp and EOI handling
o png alpha channel and BGR handling
I am very interested in helping move all required
changes into the current pre-0.92 source tree,
but have some questions, which I've outlined
below.  I would like to know whether you're 
already trying to move to 0.92, given it
doesn't contain any of the features above (yet!).
Here are my comments on each of the above, in 
more detail.
o engine/devmalloc.c STATIC memory allocations
o static buffers for images
These can be fairly easily handled by using
a specific memory allocator in all the image
routines, which could be redirected to your
STATIC_malloc() routines in devmalloc.c.

[TAO] 
This is used for avoid the memory fragment.

o NO_CHARMAP_WORKAROUND fix in font_freetype{2}.c
I'm not quite certain what this is for, it appears
to handle character values > 0x80 differently.  Can
you comment?  Also, are you currently using 
FT2, and which version?  There have been many
changes adopted in FT2 since then, and the cache
code is now working in the repository.

[TAO]
you could see HAVE_FREETYPE_2_SUPPORT=Y at the bottom of config file which will overwrite the old value.
This should be font provider related, it could be ignored.
Sometimes, it will make a little change in these files for special font.
This feature is not enabled in config.

o drivers/scr_em86xx.c and scr_smp863x.c screen drivers
Are these drivers used in WDTV, or are you using
the fblin32.c and fblin16.c drivers?

[TAO] this is the driver for sigma 863x chipset, gfx.

o MWROP_COPY_RK and MWROP_COPY_IK handling in 16 and 32bpp drivers
These should be easily added to the fblin16 and fblin32 drivers.


o TRANSLATE_ESC handling in kbd_tty.c
Easily added


o brightness handling (requiring float) in devopen.c
The current implementation drags in floating point.
It would be nice to have a brightness control
capability that didn't require float.

[TAO] please ignore it, the first line of "case MWPF_TRUECOLOR565" is using a return.

o chinese font and HBF handling in freetype 2
Not sure if this is still required.  Are these
mods, including the hbf.c files, still required
when using FT2 fonts?  Are you using FT2 fonts
on the device, or only hbf.c and compiled-in
arial fonts?

[TAO] HBF is a Hanzi Bitmap font.
USE_HBF_FONT in src/drivers/genfont.c (used for BIG5 font)
Maybe it is helpful in some low cost embedded devices with small size LCD.

o fast draw enhancements in devdraw.c::GdDrawImage
I like this modification.  However, in looking more
deeply, the speedup assumes a framebuffer display
and writes directly, bypassing the driver.  This
won't work using non-fb display drivers.  Thus, 
I will need to apply a bit more work so that 
your original speed improvement is held, and
the code works on any system (like X11 for test,
for instance). 
We might add a flag, PSF_HAVEFB, to indicate whether
the GdDrawImage routine can draw directly or must
go through the screen driver for this operation,
for instance.

[TAO] please note the BPP32_COLOR2PIXEL/BPP32_DRAWPIXEL, which has no switch section in it.

o jpeg setjmp and EOI handling
I see there is also setjmp handling that is 
new, for what reason?  Is the reason for the EOI
handling that some jpegs are never received in
full, and that hangs the jpeg decoder?

[TAO] When extract image from mp3 id3 tag will cause this issue if the image is uncompleted. 
setjmp could not handle the hang issue.

o png alpha channel and BGR handling
There is currently alpha channel support in
0.92RC2 that should work.  However, there
isn't support for conversion to BGR format.
Is this to guarantee quick drawing the 
GdDrawImage routine, or for another reason?
I would like a sample .png for testing.

[TAO] this png is 24bit, but with "tRNS Transparency" 
This is related to the libpng, just a special case for 24bit png (IHDR Bitdepth: 8 ). 
refer:
http://www.libpng.org/pub/png/spec/1.1/png-1.1.pdf
4.2.12 tRNS Transparency 

According to PNG_EXPAND_tRNS to build in an alpha channel using trans_value.
When the RGB is same as trans_value's RGB, it will set ALPHA to 0, otherwise it will set 0xFF.
======================
libpng-1.2.14/pngrtran.c
png_do_expand()
(row_info->color_type == PNG_COLOR_TYPE_RGB && trans_value) 
======================

BTW, png (with alpha channel) is much better for modern UI design than jpeg, PNG feature should be full supported.

I may have left out other requirements, please
let me know.
Regards,
Greg
---------------------------------------------------------------------
To unsubscribe, e-mail: ####@####.####
For additional commands, e-mail: ####@####.####

On Sun, Feb 21, 2010 at 06:57:16PM -0700, Greg Haerr wrote:
> We might add a flag, PSF_HAVEFB, to indicate whether the GdDrawImage
> routine can draw directly or must go through the screen driver for
> this operation, for instance.

could this be handled inside a hardware specific subdriver?  if
non-null, use the accelerated subdriver implementation, otherwise use
generic routines...

I used this technique for tremendous line draw speedups at Frye.  the
diff is at ftp://ftp.nanogui.org/pub/microwindows/patches/hold92/frye-vs-CVS-2004-01-09.diff.bz2

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

Hi, Greg
> o png alpha channel and BGR handling
> There is currently alpha channel support in
> 0.92RC2 that should work. However, there
> isn't support for conversion to BGR format.
> Is this to guarantee quick drawing the 
> GdDrawImage routine, or for another reason?
> I would like a sample .png for testing.
 
image_png.c:GdDecodePNG() has some inconsistent part. ( it works well, but inconsistent. )
In spite of MWIMAGEHDR that returned from image_png.c:GdDecodePNG() has RGB RGB RGB ...,
MWIMAGEHDR's compression field has not MWIMAGE_RGB flag.( only if alpha channel exist.)
 
I attach modified diff file.
 
--------------------------------------------------------------------------
Hi, yut616
> [TAO] this png is 24bit, but with "tRNS Transparency" 
> This is related to the libpng, just a special case for 24bit png (IHDR Bitdepth: 8 ). 
> refer:
> http://www.libpng.org/pub/png/spec/1.1/png-1.1.pdf
> 4.2.12 tRNS Transparency 
> According to PNG_EXPAND_tRNS to build in an alpha channel using trans_value.
> When the RGB is same as trans_value's RGB, it will set ALPHA to 0, otherwise it will set 0xFF.
> ======================
> libpng-1.2.14/pngrtran.c
> png_do_expand()
> (row_info->color_type == PNG_COLOR_TYPE_RGB && trans_value) 
> ======================
I guess that 0.92RC2 do support that. 
I think that the following code work for that. ( image_png.c:GdDecodePNG() )
============================================
if (png_get_valid(state, pnginfo, PNG_INFO_tRNS)) // if 'trans_value' exist, then this condition is true.
 png_set_tRNS_to_alpha(state);
============================================
 
But, I am not sure. Can you suggest some sample png? 
 
Regards.

> image_png.c:GdDecodePNG() has some inconsistent part. ( it works well, but 
> inconsistent. )
> In spite of MWIMAGEHDR that returned from image_png.c:GdDecodePNG() has 
> RGB RGB ...,
> MWIMAGEHDR's compression field has not MWIMAGE_RGB flag.( only if alpha 
> channel exist.)

Mr Sung Joo -

I've applied your patch that cleans up the ARGB/ABGR image
byte order consistency when drawing images with alpha in
GdDrawImage().  I've also added quite a few comments.
Everything is in the git repository.  Please test that the
code still works well on  16bpp 565 and 555 systems.
Thanks alot!

Since you're using C bitfields for the alpha blending, the original
code would not have worked on big endian machines.  I have
added (untested) code that should work.  It would be nice if
someone could test this by running nxviewalpha.sh which
displays an .png file with alpha, on a big endian implementation.

Also, the png fixes for WDTV should be working at this
point, see below for the original descriptions.  The png
decoder will create an alpha channel if a transparent
color was specified for the image, and GdDrawImage
will not draw the transparent portions of the image.
I'm still working on the WDTV drawing patch for
GdDrawImage that draws directly to the framebuffer.
This code, while fast, should be in a driver rather
than the engine code.

Regards,

Greg


> [TAO] this png is 24bit, but with "tRNS Transparency"
> This is related to the libpng, just a special case for 24bit png (IHDR 
> Bitdepth: 8 ).
> 4.2.12 tRNS Transparency

> According to PNG_EXPAND_tRNS to build in an alpha channel using 
> trans_value.
> When the RGB is same as trans_value's RGB, it will set ALPHA to 0, 
> otherwise it will set 0xFF.
> ======================
> libpng-1.2.14/pngrtran.c
> png_do_expand()
> (row_info->color_type == PNG_COLOR_TYPE_RGB && trans_value)
> ======================

I guess that 0.92RC2 do support that.
I think that the following code work for that.  ( 
image_png.c:GdDecodePNG()  )
============================================
if (png_get_valid(state, pnginfo, PNG_INFO_tRNS))    // if 'trans_value' 
exist, then this condition is true.
     png_set_tRNS_to_alpha(state);

> 1. devdraw better performance
in include\device.h , it defines several macros:
MACRO_DRAWPIXEL, BPP32_DRAWPIXEL  etc.
in engine\devdraw.c, GdDrawImage, it has performance issue.
in git source code:
===================
} else if (bpp > 8) { /* 16, 18, 24, or 32bpp*/
  while (height > 0) {
===================
It will check the switch condition in this "while (height > 0)"  loop 
frequently, which has performance issue.

> Maybe you could merge wdtv code to git, this is a critical issue.


YuTao -

I wanted to let you know I haven't forgotten about
this critical speedup issue.  I have been working
on getting some other important changes in place
first, before moving to optimize the GdDrawImage
routine and removing the switch condition from
the inner loop.  I am looking into what
code is generated in general by gcc in several
cases when (possibly unsorted) switch statements
are used.

Your original WDTV code performs a good optimization
using macros, which we will use.  However, it also
draws then directly to the framebuffer from the
engine layer, which means the code won't work on
X11 or other systems not using direct fb access.
I am still considering exactly how to speed this
up.  The right way is to merge the DrawArea, Blit,
and DrawImage engine routines with the screen driver
DrawArea and Blit routines, but that's a lot of work,
and we need a case that works when the screen driver
doesn't have these entry points as well.

Regards,

Greg