DiskMemImageGraphics clip issue

[Ákos Maróy]

Hi,

I encountered an issue with the DiskMemImageGraphics class, which involves clipping. I encountered this issue at first when rendering SVGs using batik to a DiskMemImage, but I seem to have managed to extract the specific issue so that there are no external dependencies.

Basically the issue is that when DiskMemImageGraphics.clip() is called, this clip is applied without taking the already set AffineTransform into account. While Graphics2D says that clipping is specified in user space, and should be applied the graphics object's transformation, in MemDiskImageGraphics' case, this does not happen.

if you take documentation of Graphics2D.clip(Shape): http://docs.oracle.com/javase/7/docs/api/java/awt/Graphics2D.html#clip(java.awt.Shape)

it says:

Intersects the current Clip with the interior of the specified Shape and sets the Clip to the resulting intersection. The specified Shape is transformed with the current Graphics2D Transform before being intersected with the current Clip.


But it seems that DiskMemImageGraphics does not translate the clipping region to the target region.

Do demonstrate the issue, see the sample code here: http://pastebin.com/AYhgWkMn (sorry, it's a bit messy). the transform & size values are taken from a specific SVG rendering trace I first encountered the issue with.

when running the above sample using a BufferedImage, the rectangle specified in the code is drawn on the target image, and the output is:

s bounds: java.awt.Rectangle[x=343,y=502,width=97,height=84]
grr transform: AffineTransform[[0.318755336305853, 0.0, 310.40200361261435], [0.0, 0.318755336305853, 85.32332454819294]]
grr clip: java.awt.geom.Rectangle2D$Float[x=343.92856,y=502.51584,w=96.0,h=83.0]
grr clip bounds: java.awt.Rectangle[x=343,y=502,width=97,height=84]
intersects: true


now, if the same code is run using a MemDiskImageGraphics object (uncomment / comment to 'select'), the rectangle does not appear on the target image, and the output is:

s bounds: java.awt.Rectangle[x=343,y=502,width=97,height=84]
grr transform: AffineTransform[[0.318755336305853, 0.0, 310.40200361261435], [0.0, 0.318755336305853, 85.32332454819294]]
grr clip: java.awt.geom.Area@12f64b73
grr clip bounds: java.awt.Rectangle[x=0,y=0,width=96,height=83]
intersects: false



one can clearly see the difference: in one case, the clip area has been translated, so that it coincides with the translated rectangle area, in the second case, it has not.


the reason I put the intersect() test into the code is that batik uses the same test to see if a shape should be drawn - and as a result, it will not draw some SVG shapes, as this test fails. (for people interested, the test is at org.apache.batik.gvt.AbstractGraphicsNode.paint(), line 491.)


when looking at the source code of BufferedImage.clip() (see for example here: http://javasourcecode.org/html/open-source/jdk/jdk-6u23/java/awt/Graphics2D.java.html ), it's clear that there they make an effort to translate the clip region into device space, and 'untranslate' it whet getClip() needs to return the clip region. I wonder if something similar should be done in the case of MemDiskImageGraphics.


Akos

[Michael Bedward]

Hi Akos,

Thanks for this. I've just run your sample code here with
BufferedImage, TiledImage and DiskMemImage and confirm that the first
two classes produce equivalent results (I see two sides of the
rectangle) while with DiskMemImage the rectangle is not drawn.

I agree that the behaviour of our image and graphics classes should
conform to that of the java.awt classes unless there is a very good
reason not to. In this case it seems that the difference of behaviour
is simply due to oversight on my part rather than design.
Previous work on clipping was very limited - I can only find this:

https://github.com/mbedward/jaitools/issues/132

So let's fix it ! It would be great to get this working properly for
the next release (which I'm hoping will be next month).

Here is an issue:
https://github.com/mbedward/jaitools/issues/219

If you have time to look into it further and prepare a patch or a pull
request that would be great. Otherwise I will add it to my to-do list
but won't be able to look at it for a week or two.

Michael

> --
> You received this message because you are subscribed to the Google Groups
> "jai-tools" group.
> To post to this group, send email to jai-...@googlegroups.com.
> To unsubscribe from this group, send email to
> jai-tools+...@googlegroups.com.
> For more options, visit this group at
> http://groups.google.com/group/jai-tools?hl=en.

[Ákos Maróy]

Michael,

> Thanks for this. I've just run your sample code here with
> BufferedImage, TiledImage and DiskMemImage and confirm that the first
> two classes produce equivalent results (I see two sides of the
> rectangle) while with DiskMemImage the rectangle is not drawn.

I'm glad you could reproduce the issue as well!

OTOH, how I see TiledImage working is that while it will draw the
rectangle, it will also report non-translated clipping bounds:

s bounds: java.awt.Rectangle[x=343,y=502,width=97,height=84]
grr transform: AffineTransform[[0.318755336305853, 0.0,
310.40200361261435], [0.0, 0.318755336305853, 85.32332454819294]]

grr clip: java.awt.geom.Area@589af639

grr clip bounds: java.awt.Rectangle[x=0,y=0,width=96,height=83]
intersects: false

this means that batik wouldn't draw the image using a TiledImage either,
as it's testing for the intersection of the clip region and the shape to
be drawn.
(I raised this issue on the batik mailing list, but they didn't yet respond)

> So let's fix it ! It would be great to get this working properly for
> the next release (which I'm hoping will be next month).
>
> Here is an issue:
> https://github.com/mbedward/jaitools/issues/219
>
> If you have time to look into it further and prepare a patch or a pull
> request that would be great. Otherwise I will add it to my to-do list
> but won't be able to look at it for a week or two.
>

I'd be glad to look into it. would you have a suggestion on which way to go

when looking at the source code of Graphics2D source code, it seems that
they have a usrClip property, which they store in a translated shape,
and 'untranslate' it for a getClip() call. do you think this would be
the way to go?


Akos

[Michael Bedward]

On 23 January 2013 19:39, Ákos Maróy <ak...@maroy.hu> wrote:
>
> OTOH, how I see TiledImage working is that while it will draw the
> rectangle, it will also report non-translated clipping bounds:
>

> this means that batik wouldn't draw the image using a TiledImage either,
> as it's testing for the intersection of the clip region and the shape to
> be drawn.
> (I raised this issue on the batik mailing list, but they didn't yet respond)
>

Bummer - I didn't look closely enough at the output when I ran your
code. So we have three different behaviours, with two of them courtesy
of Sun/Oracle (sigh).

I'm not familiar with batik and (as the code shows) haven't through
about these issues properly, but the example code bahaviour with
BufferedImage / SunGraphics2D seems least surprising, which implies
that we treat TileImageGraphics as broken...

> I'd be glad to look into it. would you have a suggestion on which way to go
>
> when looking at the source code of Graphics2D source code, it seems that
> they have a usrClip property, which they store in a translated shape,
> and 'untranslate' it for a getClip() call. do you think this would be
> the way to go?

It sounds reasonable - there's already so much state in that class I
guess adding more isn't such a sin. Another option would be to hold
both the transformed and untransformed shape.

Michael

[Michael Bedward]

"haven't through about these issues properly" == haven't _thought_
about these issues properly :)

[Ákos Maróy]

On 23/01/13 11:13, Michael Bedward wrote:

It sounds reasonable - there's already so much state in that class I guess adding more isn't such a sin. Another option would be to hold both the transformed and untransformed shape. Michael

I attached a suggested patch to the ticket, see also here:

https://gist.github.com/4612129

the raster comparison test fails for me, because the image drawn on the DiskMemImage and BufferedImage are slightly different - one is about 1 pixel higher than the other one. this may be due to rounding errors? you can also visual compare the two images, which are created in /tmp

otherwise, they seem to work OK

[Michael Bedward]

Hi Akos,

> I attached a suggested patch to the ticket, see also here:
>
> https://gist.github.com/4612129

Thanks for the careful work. Looks good.

I'll do some tests locally later today (Sydney time). In particular, I
want to check that all is still ok when the private copyGraphicsParams
method is called with a non-null Point argument to translate the clip
area.

> the raster comparison test fails for me, because the image drawn on the
> DiskMemImage and BufferedImage are slightly different - one is about 1 pixel
> higher than the other one. this may be due to rounding errors? you can also
> visual compare the two images, which are created in /tmp

OK - I'll try to make that test a little looser to allow for rendering
differences between systems. We could also add an option to use
perceptualdiff if it is installed on the host system (as Andrea has
done with GeoTools tests).

Michael

[Michael Bedward]

Hi Akos,

I've looked at the code a little more. The only problem I've found so
far is that the clip method was missing a check for a null Shape arg
(clear clip region). I'm still a little concerned about how the
changes interact with the existing transform code in the doDraw method
- but won't touch this unless tests or drawing results suggest a
problem.

Since I'm sure that your changes are a big improvement over the prior
code, I've pushed them to master and will update the snapshots
shortly. For the moment I've put an Ignore on ClipTest - I'll look at
fixing it up over the next day or two. We also need to add some basic
tests for getClip etc.

I've updated the demo program
(org.jaitools.demo.tiledimage.DrawingDemo) to show a simple clipping
with transform example.

Michael

[Ákos Maróy]

Michael,

it's great you've had some time looking at my patch

> I've looked at the code a little more. The only problem I've found so
> far is that the clip method was missing a check for a null Shape arg
> (clear clip region).

I'm sorry for this mistake

> I'm still a little concerned about how the
> changes interact with the existing transform code in the doDraw method
> - but won't touch this unless tests or drawing results suggest a
> problem.

yes, this is a concern. for start, it's a good thing that all unit tests
pass, but when it comes to rendering that's never enough

actually, I've managed to make a considerably sized & complex rendering
just after finishing the patch, a 1:500.000 scale 300dpi rendering of
Hungary, including generic OpenStreetMap data and aviation related
information, see here:
http://openaviationmap.tyrell.hu/oam_hungary_500000.tif (caveat: it's
400MB). all looks pretty OK from a rendering perspective.

then again, this is also just one sample that seems to work.

> Since I'm sure that your changes are a big improvement over the prior
> code, I've pushed them to master and will update the snapshots
> shortly.

thank you!

> For the moment I've put an Ignore on ClipTest - I'll look at
> fixing it up over the next day or two. We also need to add some basic
> tests for getClip etc.

indeed. let me know if you have specific suggestions, and I can help out
with these as well.

> I've updated the demo program
> (org.jaitools.demo.tiledimage.DrawingDemo) to show a simple clipping
> with transform example.
>

wow, excellent

thanks for quickly incorporating my patch into the codebase!


Akos

[Michael Bedward]

Hi Akos,

> actually, I've managed to make a considerably sized & complex rendering
> just after finishing the patch, a 1:500.000 scale 300dpi rendering of
> Hungary, including generic OpenStreetMap data and aviation related
> information, see here:
> http://openaviationmap.tyrell.hu/oam_hungary_500000.tif (caveat: it's
> 400MB). all looks pretty OK from a rendering perspective.

Fantastic ! It looks really good (I couldn't resist downloading it to
check it out)

>> For the moment I've put an Ignore on ClipTest - I'll look at
>> fixing it up over the next day or two. We also need to add some basic
>> tests for getClip etc.
> indeed. let me know if you have specific suggestions, and I can help out
> with these as well.

Don't worry - I will :)

> thanks for quickly incorporating my patch into the codebase!

No problems. The updated snapshot jars are going up the line now.

Aside: it would be great to get a post about your large image
rendering for either the GeoTools or JAITools blogs at some stage.

Michael

[Ákos Maróy]

On 24/01/13 08:18, Michael Bedward wrote:
> Fantastic ! It looks really good (I couldn't resist downloading it to
> check it out)

I'm glad you like it

>> indeed. let me know if you have specific suggestions, and I can help out
>> with these as well.
> Don't worry - I will :)

thank you!

>
>> thanks for quickly incorporating my patch into the codebase!
> No problems. The updated snapshot jars are going up the line now.
>
> Aside: it would be great to get a post about your large image
> rendering for either the GeoTools or JAITools blogs at some stage.
>

sure, I will clean the rendering code in the coming days and commit it
to the OpenAviationMap repository. the code itself by now is quite
short, so it can serve as a good sample for anyone who'd want to do
something similar. or would you want me to write a blog post on this topic?

[Andrea Aime]

On Thu, Jan 24, 2013 at 7:57 AM, Ákos Maróy <ak...@maroy.hu> wrote:
> actually, I've managed to make a considerably sized & complex rendering
> just after finishing the patch, a 1:500.000 scale 300dpi rendering of
> Hungary, including generic OpenStreetMap data and aviation related
> information, see here:
> http://openaviationmap.tyrell.hu/oam_hungary_500000.tif (caveat: it's
> 400MB). all looks pretty OK from a rendering perspective.

Nicely done :)
Out of curiosity, how much memory did you give the VM to generate it?
How much time did it take?

Cheers
Andrea

--
==
Our support, Your Success! Visit http://opensdi.geo-solutions.it for
more information.
==

Ing. Andrea Aime
@geowolf
Technical Lead

GeoSolutions S.A.S.
Via Poggio alle Viti 1187
55054 Massarosa (LU)
Italy
phone: +39 0584 962313
fax: +39 0584 1660272
mob: +39 339 8844549

http://www.geo-solutions.it
http://twitter.com/geosolutions_it

-------------------------------------------------------

[Ákos Maróy]

On 24/01/13 09:51, Andrea Aime wrote:

> On Thu, Jan 24, 2013 at 7:57 AM, �kos Mar�y <ak...@maroy.hu> wrote:
>> actually, I've managed to make a considerably sized & complex rendering
>> just after finishing the patch, a 1:500.000 scale 300dpi rendering of
>> Hungary, including generic OpenStreetMap data and aviation related
>> information, see here:
>> http://openaviationmap.tyrell.hu/oam_hungary_500000.tif (caveat: it's
>> 400MB). all looks pretty OK from a rendering perspective.
> Nicely done :)

thank you :)

> Out of curiosity, how much memory did you give the VM to generate it?
> How much time did it take?
>

I used -Xmx8G , with -Xmx6G it would run out of memory (I guess it does
load all features into memory at some point)

took between 20-30 minutes, on an Ubuntu 12.10 64 bit, intel i7 3.2GHz,
12GB RAM, slow (encrypted) hard disk

[Andrea Aime]

On Thu, Jan 24, 2013 at 9:59 AM, Ákos Maróy <ak...@maroy.hu> wrote:
>> Out of curiosity, how much memory did you give the VM to generate it?
>> How much time did it take?
>>
> I used -Xmx8G , with -Xmx6G it would run out of memory (I guess it does
> load all features into memory at some point)

Hmm... labels need to be kept in memory for the conflict resolution algorithm
to work.
Actually, if the labelling does not use the "priority" option the labeller could
also be made to stream.

One thing to also take into consideration is that if you have
multiple feature type
styles in your map (to force some Z ordering) the code will instantiate
BufferedImages to avoid loading features multiple times.

This can be disabled using the following, that will make the code load features
multiple times instead:

Map<String, Object> rendererParams = new HashMap<String, Object>();
rendererParams.put("optimizedDataLoadingEnabled", Boolean.TRUE);
rendererParams.put(StreamingRenderer.OPTIMIZE_FTS_RENDERING_KEY,
Boolean.FALSE);
renderer.setRendererHints(rendererParams);
renderer.setMapContent(mapContent);

Hope this helps

[Ákos Maróy]

On 24/01/13 10:15, Andrea Aime wrote:

Hmm... labels need to be kept in memory for the conflict resolution algorithm
to work.
Actually, if the labelling does not use the "priority" option the labeller could
also be made to stream.

but, well, I do need conflict resolution :)

One thing to also take into  consideration is that if you have
multiple feature type
styles in your map (to force some Z ordering) the code will instantiate
BufferedImages to avoid loading features multiple times.

I'm not sure I understand what you mean

This can be disabled using the following, that will make the code load features
multiple times instead:

        Map<String, Object> rendererParams = new HashMap<String, Object>();
        rendererParams.put("optimizedDataLoadingEnabled", Boolean.TRUE);
        rendererParams.put(StreamingRenderer.OPTIMIZE_FTS_RENDERING_KEY,
Boolean.FALSE);
        renderer.setRendererHints(rendererParams);
        renderer.setMapContent(mapContent);

will look into it, thanks


BTW, here is the code that does the rendering: http://code.google.com/p/openaviationmap/source/browse/trunk/oam-java-tools/src/main/java/hu/tyrell/openaviationmap/rendering/RenderMap.java