Ticket #2173 (closed defect: fixed)

Opened 6 years ago

Last modified 6 years ago

AGG and GD alpha values are incompatible

Reported by: sdlime Owned by: sdlime
Priority: normal Milestone: 5.0 release
Component: AGG Version: 5.0
Severity: normal Keywords:
Cc: tbonfort, dmorissette

Description

AGG and GD, it turns out, interpret alpha channel values totally opposite one another. AGG does it correctly, while GD is backwards (and uses a 128 value range for some reason).

We need to work around this limitation.

Attachments

pie-r6378.png Download (55.2 KB) - added by tbonfort 6 years ago.
pie-r6387.png Download (53.6 KB) - added by tbonfort 6 years ago.
pie.map Download (1.1 KB) - added by tbonfort 6 years ago.
mapfile used for previous images
agg2gd.diff Download (3.9 KB) - added by tbonfort 6 years ago.
agg2gd.2.diff Download (4.5 KB) - added by tbonfort 6 years ago.
gd2agg.png Download (3.7 KB) - added by tbonfort 6 years ago.
rendering bug with special gd values

Change History

Changed 6 years ago by sdlime

Revision 6387 features a fix that resets the alpha channel for the image after rendering vector or chart layers using AGG. I view this as only temporary but it solves the text rendering issue described by Steve Woodbridge.

I think a more permanent solution involves (from the AGG users list):

| Hi all: I'm using AGG to write to a 32-bit rgba buffer managed by another library. | Is it possible to have AGG render only in the rgb channels and leave the alpha | channel unaltered?

Yes, you just need to use pixfmt_rgb24 pixel format (24-bit values), but in rendering_buffer class set the stride to width * 4 (32-bit values).

From  http://www.antigrain.com/doc/basic_renderers/basic_renderers.agdoc.html#toc0001

stride — The “stride” (big step) of the rows measured in objects of type T. Class rendering_buffer is “typedefed” as row_ptr_cache<int8u>, so, this value is in bytes. Stride determines the physical width of one row in memory. If this value is negative the direction of the Y axis is inverted, that is, Y==0 will point to the last row of the buffer, Y==height-1 — to the first one. The absolute value of stride is important too, because it allows you to work with buffers whose rows are aligned in memory (like in Windows BMP, for example). Besides, this parameter allows you to work with any rectangular area inside the buffer as with the whole buffer.

Changed 6 years ago by tbonfort

changeset r6387 introduces artifacts in rendering - see attachments pie-r6387.png and pie-r6378.png
this is drawing a 100-opacity layer over another 100-opacity one

Changed 6 years ago by tbonfort

Changed 6 years ago by tbonfort

Changed 6 years ago by tbonfort

mapfile used for previous images

Changed 6 years ago by tbonfort

  • cc tbonfort added

Changed 6 years ago by sdlime

  • status changed from new to assigned

Back to the drawing board on this one. What's funny is that the change is done AFTER the rendering. Do you have a test chart dataset I could have access to?

Steve

Changed 6 years ago by tbonfort

Changed 6 years ago by tbonfort

Changed 6 years ago by tbonfort

(please ignore agg2gd.diff forget to check that replace button)
included patch to convert alpha values from/to agg and gd. these functions are called before and after a call to a function that uses agg to render on an image

the patch also includes a fix to msImageCopyMergeNoAlpha that takes into account the alpha values of the src image when blending it onto the destination one (the name of the function doesn't really make any sense now):
- agg layers are rendered correctly with or without transparency enabled
- gd rendering of thick antialiased lines is now correct

Changed 6 years ago by sdlime

The msImageCopyMergeNoAlpha() function was added when AGG values were absolutely incompatible with GD so it ignored them altogether. We should compare the patched version with msImageCopyMerge() (also in mapgd.c). It may be that we can combine to one function now.

The patch has been applied (rev. 6408).

Two test cases to try:

  • AGG over GD-based raster. I think we may need to convert from AGG2GD alpha values prior to msDrawRasterLayer().
  • label cache with AGG. I think that should be fixed, but need to check.

Thomas, thanks for all the great work on this!

Steve

Changed 6 years ago by sdlime

  • status changed from assigned to closed
  • resolution set to fixed

I'm going to close this one now. We can open up more specific tickets going forward.

Steve

Changed 6 years ago by tbonfort

  • status changed from closed to reopened
  • resolution fixed deleted

sorry reopening :(
the gd alpha values are 0 to 127 so that negative colors (i.e. with alpha>127) can be treated as special cases (gdTiled and so on).
the msAlphaGD2AGG has calls to gdImageSetPixel which treats these special values. when a gd alpha is transformed to something which hits these values, rendering artifacts appear (see black dots in attached file gd2agg.png)

fix: in mapagg.cpp, in function msAlphaGD2AGG at line 1141, replace

gdImageSetPixel(im->img.gd, x, y, ((c)&0x00FFFFFF)|(alpha<<24));

with

(im->img.gd)->tpixels[y][x] = ((c)&0x00FFFFFF)|(alpha<<24);

Changed 6 years ago by tbonfort

rendering bug with special gd values

Changed 6 years ago by dmorissette

  • cc dmorissette added
  • status changed from reopened to closed
  • resolution set to fixed

Thomas wanted to make sure that this made it in today's beta2, so I have committed a modified version of his latest patch in r6433 that uses the gdImageTrueColorPixel() macro instead of accessing tpixels[][] directly.

Marking fixed.

Changed 6 years ago by sdlime

I read the change as switching from the macro to the direct array access. Am I wrong? - Steve

Changed 6 years ago by tbonfort

gdImageSetPixel isn't a macro

gdImageTrueColorPixel is

Changed 6 years ago by sdlime

I see now. I didn't see any reference to the macro in the bug which is why I got confused... Steve

Note: See TracTickets for help on using tickets.