Opened 15 years ago

Closed 14 years ago

Last modified 14 years ago

#2173 closed defect (fixed)

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 (6)

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

Download all attachments as: .zip

Change History (18)

comment:1 Changed 15 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.

comment:2 Changed 15 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 15 years ago by tbonfort

Attachment: pie-r6378.png added

Changed 15 years ago by tbonfort

Attachment: pie-r6387.png added

Changed 15 years ago by tbonfort

Attachment: pie.map added

mapfile used for previous images

comment:3 Changed 15 years ago by tbonfort

Cc: tbonfort added

comment:4 Changed 15 years ago by sdlime

Status: newassigned

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 15 years ago by tbonfort

Attachment: agg2gd.diff added

Changed 15 years ago by tbonfort

Attachment: agg2gd.2.diff added

comment:5 Changed 15 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

comment:6 Changed 15 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

comment:7 Changed 14 years ago by sdlime

Resolution: fixed
Status: assignedclosed

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

Steve

comment:8 Changed 14 years ago by tbonfort

Resolution: fixed
Status: closedreopened

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 14 years ago by tbonfort

Attachment: gd2agg.png added

rendering bug with special gd values

comment:9 Changed 14 years ago by dmorissette

Cc: dmorissette added
Resolution: fixed
Status: reopenedclosed

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.

comment:10 Changed 14 years ago by sdlime

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

comment:11 Changed 14 years ago by tbonfort

gdImageSetPixel isn't a macro

gdImageTrueColorPixel is

comment:12 Changed 14 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.