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

Screenshot Command #98

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open

Conversation

landersson
Copy link
Contributor

First take on a remote command for saving a screenshot of the current view.

@landersson landersson mentioned this pull request Mar 21, 2016
@c42f
Copy link
Owner

c42f commented Mar 21, 2016

Thanks a lot, the general structure of this looks great.

I think there's a bit of work left to do to make sure that taking screenshots doesn't effect the GUI and OpenGL state, in particular, the camera handling. Will have a closer look through the code.

Also I'm not sure about controlling what gets rendered into the screenshot. In principle you might want to render bounding boxes, meshes, cursor, etc. Probably the easiest thing is to take this state from the GUI so that the rendered screenshot resembles the GUI as much as possible (see also the camera location).

width *= dPR;
height *= dPR;

QOpenGLFramebufferObject framebuffer(QSize(width, height), QOpenGLFramebufferObject::Depth);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This class is Qt5 only, so we'd be breaking with Qt4 compatibility by using this. I guess let's get it all working, and then I'll need to decide.

(TBH, I'm kinda keen to give up on Qt4 because the number of different OpenGL and OS versions is already enough to be a real drag on limited development resources...)

@c42f
Copy link
Owner

c42f commented Mar 21, 2016

Ok, I've had a look over things. Generally looks good, other than some remaining entanglement between GUI state and offscreen rendering, plus the Qt5 issue.

@landersson
Copy link
Contributor Author

Just trying to use this to get some rendering done by repeatedly issuing the "-viewposition" and "-screenshot" commands from a python script, but it's pretty slow, I guess mainly because both of these commands will issue an unnecessary, complete gui redraw... I guess something that would solve the problem for me would be to send position and view angles as 6, probably optional, numeric parameters to the screenshot command.

@c42f
Copy link
Owner

c42f commented Mar 22, 2016

How many points do you have? There's a few things which can cause it to be slow:

  • Time to draw a complete frame. If this is 1e8 points or so this could be several seconds.
  • IPC overhead due to opening a new socket connection for each request. This can introduce a quite a bit of latency (perhaps 50ms to open the socket? Can't remember)
  • As you said, forcing GUI redraws could make things twice as expensive.

The solution will be quite different depending on what the problem is.

@landersson
Copy link
Contributor Author

My biggest cloud is about 5e8 points. It's currently decimated by 3, but I was planning to use full density when rendering the final sequence. So yes, I guess it's not too surprising that it'll take a while to render all those points. The IPC overhead should be negligible in my case. I'll try to change things so an off screen render doesn't trigger a GUI redraw. Btw, just noticed that the -maxpoints command doesn't seem to work. It doesn't update the FileLoader object's internal max count setting which get's set when the FileLoader is instantiated.

@c42f
Copy link
Owner

c42f commented Mar 29, 2016

Thanks for pointing out the problem with -maxpoints, I've got a mostly-written patch for that which I'll try to get committed in the next couple of days.

@landersson
Copy link
Contributor Author

No problem. I'm still planning to get this pull request into a mergeable state at some point btw, but it might be a while until I have enough time to do it properly.

@jkwashbourne
Copy link

jkwashbourne commented Dec 14, 2021

Im guessing this PR is abandoned? Is there any thought towards implementing the off screen anti-aliased screen shot command? Thanks much for your time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants