New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fixed patch from #4834 to fix transparency in animated gif #595

Closed
wants to merge 3 commits into
base: master
from

Conversation

Projects
None yet
3 participants
@rduivenvoorde
Contributor

rduivenvoorde commented May 26, 2014

this is actually the patch from Simone Giannecchini, with one little
fix in RenderedImageMapResponse to make it work (again)

see: https://jira.codehaus.org/browse/GEOS-4834

I'm not aware of tests which can test this automatically, but if possible please guide me and I will try to add tests for this.

I can create a simple shapefile and url's to test this, if needed.
This work is related to https://jira.codehaus.org/browse/GEOS-6479 for which this shapefile also could be used then.
OR is there already a time/elevation related test shape in the demo data?

fixed patch from #4843 to fix transparency in animaged gif
this is actually the patch from Simone Giannecchini, with one little
fix in RenderedImageMapResponse to make it work (again).

see: https://jira.codehaus.org/browse/GEOS-4834

@rduivenvoorde rduivenvoorde changed the title from fixed patch from #4843 to fix transparency in animaged gif to fixed patch from #4843 to fix transparency in animated gif May 26, 2014

@aaime

This comment has been minimized.

Member

aaime commented May 26, 2014

Hi, thanks a lot for the work.

In terms of adding a test, you should be able to extend or mimic DimensionVectorGetMapTest.testTimeListAnimated, and parse back the response to see if there are transparent pixels, or not.

Also, I see you modified two files compared to Simone's patch, so you should sign and send by mail the contribution agreement: geoserver.org/comm/assignment_agreement.pdf

@rduivenvoorde

This comment has been minimized.

Contributor

rduivenvoorde commented Jun 2, 2014

@aaime sorry, I'm trying to understand how the test classes are setup, so I can check which pixels are supposed to be transparent or not.

My plan was to use available testdata to set up a local geoserver and to fire the url's from for example: https://github.com/geoserver/geoserver/blob/master/src/wms/src/test/java/org/geoserver/wms/wms_1_1_1/DimensionsVectorGetMapTest.java
And then check for transparency as you say. But to determine which pixels should actually be transparent at given timeframe, I think I need an image first?

But I seem not to be able to find the data which is actually used there.
Can you give me a hint?

@rduivenvoorde

This comment has been minimized.

Contributor

rduivenvoorde commented Jun 23, 2014

Hi @aaime,

I added two tests now. Please try/test/review :-)
Actually checking the whole animated image can only be done visually, but now I put the frames on a 'filmstrip' and check there the transparency.

I commented some writing to image files in the code, but I'll add the wanted images below.

I hope it is ok like this.

geoserveranimatednontransparentframe0
geoserveranimatednontransparentframe1
geoserveranimatednontransparentframe2
geoserveranimatedstripnontransparent
geoserveranimatedstriptransparent
geoserveranimatedtransparentframe0
geoserveranimatedtransparentframe1
geoserveranimatedtransparentframe2

@dromagnoli

This comment has been minimized.

Contributor

dromagnoli commented Jul 16, 2014

Hi Richard,
did you sign and send the contribution agreement referred by Andrea?

@rduivenvoorde

This comment has been minimized.

Contributor

rduivenvoorde commented Jul 16, 2014

Hi Daniele

yes I actually did that immediately, I sent it on the 26th of may to: assignment@openplans.org, emcdermott@openplans.org and cc to andrea.aime@geo-solutions.it

have just forwarded the message to simone.giannecchini@geo-solutions.it as questioned.

plz let me know if you need more information

@dromagnoli

This comment has been minimized.

Contributor

dromagnoli commented Jul 16, 2014

Hi Richard,
I have squashed your pull request into a pull-request containing a single commit instead of 3.
It is available here:
#649

We will merge it very soon.
I'm closing this one

@dromagnoli dromagnoli changed the title from fixed patch from #4843 to fix transparency in animated gif to fixed patch from #4834 to fix transparency in animated gif Jul 17, 2014

@dromagnoli dromagnoli closed this Jul 17, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment