JuliaGL / GLVisualize.jl

Visualization library written in Julia and OpenGL
Other
247 stars 34 forks source link

Make deleting an object easier by returning the output of extract_renderable from _view #243

Closed mohamed82008 closed 6 years ago

mohamed82008 commented 6 years ago

I have been trying to delete and add meshes at will but couldn't find a function of delete! that just accepts a HomogeneousMesh or a GLAbstraction.Context which is the output of visualize. The problem is that the delete! function accepts only GLAbstraction.RenderObjects and the only way I could find to return such a representation was using extract_renderable which returns a vector of GLAbstraction.RenderObjects.

This is all fine but given that _view calls internally extract_renderable in one of its methods, and _view itself returns nothing, I think it would convenient to return the vector of render objects from _view which I can then call with delete!.(window, robjects) when I am done with that mesh or want to replace it. Does this sound like a welcome PR? (It's actually a very simple change from line 68 in visualize_interface.jl)

    robj
end

_view(robjs::Vector, screen = current_screen(); kw_args...) = #for robj in robjs
    _view.(robjs, screen; kw_args...)
#end
SimonDanisch commented 6 years ago

Yeah why not ;) I changed this, because it printed so much when using it interactively in the REPL. But I think that shouldn't be an issue anymore! But please return the Context! I added this in GLWindow: https://github.com/JuliaGL/GLWindow.jl/blob/master/src/screen.jl#L529 to delete a Context.

mohamed82008 commented 6 years ago

Oh I didn't know about this function! Then this PR might be less useful than I thought. It will only be useful if someone runs:

_view(visualize(colored_mesh))

so returning the input to _view gives a handle for deleting the Composable but I might as well separate it into 2 lines!

To return the Composable when input, only the following change needs to be made to the last line of the same file:

_view(c::Composable, screen = current_screen(); kw_args...) = begin; _view(extract_renderable(c), screen; kw_args...); return c; end;

I might even use the Github pen for that!

mohamed82008 commented 6 years ago

Actually nevermind, making this change will make _view inconsistent for different inputs, which doesn't sound too attractive. I think separating the call into 2 lines makes more sense and is more elegant.