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

we used to have two path for rendering a frame #2028

Merged
merged 3 commits into from Jun 22, 2016

Conversation

Projects
None yet
3 participants
@doutriaux1
Member

doutriaux1 commented Jun 14, 2016

the one saving pngs was broken and not preserving pngs
two paths are now unified
this also fixes zoom on animations.
fix #1845


This change is Reviewable

we used to have two path for rendering a frame
the one saving pngs was broken and not preserving pngs
two paths are now unified
this also fixes zoom on animations.
fix #1845
@doutriaux1

This comment has been minimized.

Member

doutriaux1 commented Jun 14, 2016

@doutriaux1

This comment has been minimized.

Member

doutriaux1 commented Jun 14, 2016

@aashish24

This comment has been minimized.

Contributor

aashish24 commented Jun 14, 2016

How do we test it @doutriaux1 ?

@doutriaux1

This comment has been minimized.

Member

doutriaux1 commented Jun 14, 2016

import vcs
import cdms2
import os

f=cdms2.open(os.path.join(vcs.sample_data,"clt.nc"))
s=f("clt",slice(0,12))

x=vcs.init(geometry=(800,600))
x.colormap = "AMIP" 
x.plot(s)       
x.animate.create(thread_it=1)
x.animate.zoom(2)
#x.animate.run()
raw_input("Press enter")
x.animate.save("crap.mp4")
@danlipsa

This comment has been minimized.

Contributor

danlipsa commented Jun 15, 2016

@doutriaux1 Do we have a test that failed before and passes with this change?

@doutriaux1

This comment has been minimized.

Member

doutriaux1 commented Jun 15, 2016

nope. and animation testing are such a pain... I guess we could edit all of our exisiting animation test and force them to have a different colormap.

@danlipsa

This comment has been minimized.

Contributor

danlipsa commented Jun 15, 2016

@doutriaux1 Probably its enough if you only change one. If its easy to do it may be worth it - if not, its fine.

@aashish24

This comment has been minimized.

Contributor

aashish24 commented Jun 15, 2016

LGTM 👍 but I didn't run testing.

@aashish24

This comment has been minimized.

Contributor

aashish24 commented Jun 22, 2016

@doutriaux1 @danlipsa can we merge this now?

@doutriaux1 doutriaux1 merged commit 9369abf into master Jun 22, 2016

0 of 2 checks passed

continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
continuous-integration/travis-ci/push The Travis CI build is in progress
Details

@doutriaux1 doutriaux1 deleted the issue_1845_animation_cmap_not_preserved branch Jun 22, 2016

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