Skip to content
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

png not respecting user dims if bigger than screen (Mac) #129

Open
doutriaux1 opened this issue Feb 23, 2017 · 19 comments
Open

png not respecting user dims if bigger than screen (Mac) #129

doutriaux1 opened this issue Feb 23, 2017 · 19 comments
Assignees
Labels
bug low Low Priority bug (but not enhancement)
Milestone

Comments

@doutriaux1
Copy link
Contributor

doutriaux1 commented Feb 23, 2017

import vcs
x=vcs.init(geometry={"width":8000,"height":4000})
x.open()
x.plot([1,2,3,4,3,2,1])
x.png("bad",width=8000,height=4000,units="pixels")

bad

@aashish24
Copy link
Contributor

@doutriaux1 do you want to look into this or should someone else?

@doutriaux1
Copy link
Contributor Author

@aashish24 apparently there's nothing to do about it, short of re-running the pipeline on the bg=1 canvas when calling png. @chaosphere2112 mentioned we can't just transfer the VTK Actor to the new backend.

@aashish24
Copy link
Contributor

@chaosphere2112 mentioned we can't just transfer the VTK Actor to the new backend.

@doutriaux1 not sure if understand it correctly.

@aashish24 apparently there's nothing to do about it, short of re-running the pipeline on the bg=1

For OpenGL that is true (and I would think that for the rendering as well but may be not SVG). The underlying system would not let the window bigger than the window viewport.

@chaosphere2112
Copy link
Contributor

@aashish24 This was referencing information that you and David Lonie told me a year or two ago about my animation solution; we can't move the entire vtkRenderer stack to a new render window and expect things to function correctly.

@aashish24
Copy link
Contributor

@aashish24 This was referencing information that you and David Lonie told me a year or two ago about my animation solution; we can't move the entire vtkRenderer stack to a new render window and expect things to function correctly.

thanks @chaosphere2112 that makes sense.
actually since we are hoping to move to opengl2, this won't br an issue since if you have a vtkRenderer and you pass it to a new vtkRenderWindow it should destroy the current context objects and create new ones (typically OpenGL stuff) but i will double check on it.

@aashish24
Copy link
Contributor

@aashish24 apparently there's nothing to do about it, short of re-running the pipeline on the bg=1 canvas when calling png.

@doutriaux1 I think we can close it?

@doutriaux1
Copy link
Contributor Author

@aashish24 let's at least add a warning to the user.

@doutriaux1 doutriaux1 modified the milestone: 3.0 May 5, 2017
@doutriaux1 doutriaux1 modified the milestones: 3.0, post 3.0 Mar 29, 2018
@danlipsa
Copy link
Contributor

@scottwittenburg @doutriaux1 This happens with master after the context2d merge as well (on linux)

@danlipsa
Copy link
Contributor

danlipsa commented Dec 19, 2018

@doutriaux1 @scottwittenburg @aashish24 VTK has support for saving images with a certain magnification. That means that the window on the screen and the high resolution image should have the same ratio. The image will have higher resolution. I can add another parameter to the png function: magnification and deprecate width and height for png (issue a warning if they are used) but compute the magnification from width and height. What do you think?

@doutriaux1
Copy link
Contributor Author

@danlipsa I'm not sure I follow this. @scottwittenburg has a fix at: https://github.com/CDAT/vcs/pull/376/files if @scottwittenburg thinks your idea is better then let's update @scottwittenburg branch with it.

@danlipsa
Copy link
Contributor

I think it makes sense for the window and the png to have the same ratio - otherwise you could get something very different in the png

@doutriaux1
Copy link
Contributor Author

@danlipsa, again I'm not sure exactly about what you're suggesting (link to examples?) but for the logo, I think that the idea was that the user's png would NOT be stretched in any away, but fit into the width/height box defined. We could add an option to make the log fit exactly that user-defined box and stretch the image. Is that what you're suggesting?

@danlipsa
Copy link
Contributor

danlipsa commented Dec 19, 2018

No. That's not for the logo, but for saving a plot as a png with higher resolution than the window

@danlipsa
Copy link
Contributor

Take a look at the example at the beginning of the issue

@doutriaux1
Copy link
Contributor Author

@danlipsa ok it makes more sense now... Ok according to @aashish24 comments above now that we have the new opengl "actually since we are hoping to move to opengl2, this won't br an issue since if you have a vtkRenderer and you pass it to a new vtkRenderWindow it should destroy the current context objects and create new ones (typically OpenGL stuff) but i will double check on it."

@danlipsa I trust your judgement. Feel free to pass a PR, I think what the user wants in the end is the capability to save the png (pdf, svg, etc...) at much higher res that the window shows. e.g window is opened with 800px wide but user wants to save with 3000px wide.

@danlipsa
Copy link
Contributor

Sounds good - I think that's the best way to fix this. I'll start working on this.

@doutriaux1 doutriaux1 modified the milestones: 8.1, 8.2 Mar 27, 2019
@doutriaux1
Copy link
Contributor Author

after context2d when running (edited) script above, width of png is 8000 but height gets stuck at 1876.

When running with bg=1 I DO get a png that is 8000x4000 but the picture is cramped in the corner.

@doutriaux1
Copy link
Contributor Author

bad

@danlipsa
Copy link
Contributor

I did some work on this in December: VTK did not report the new dimensions correctly after resizing. It does now. I did not get a change to look in vcs to completely fix this.

@scottwittenburg scottwittenburg removed the medium Medium Priority Bugs label May 1, 2019
@scottwittenburg scottwittenburg added the low Low Priority bug (but not enhancement) label May 1, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug low Low Priority bug (but not enhancement)
Projects
None yet
Development

No branches or pull requests

5 participants