Skip to content

Added clear function to FrameBuffer.#190

Open
sebftw wants to merge 4 commits into
glumpy:masterfrom
sebftw:patch-4
Open

Added clear function to FrameBuffer.#190
sebftw wants to merge 4 commits into
glumpy:masterfrom
sebftw:patch-4

Conversation

@sebftw

@sebftw sebftw commented Jan 22, 2019

Copy link
Copy Markdown
Contributor

Instead of calling window.clear() when we have a framebuffer activated we should call framebuffer.clear().
Then it makes sure to clear the renderbuffers that are set.
This also fixes: #189

Instead of calling window.clear() when we have a framebuffer activated we should call framebuffer.clear().
Then it makes sure to clear the renderbuffers that are set. Of course merging this change will make my other pull request redundant:
glumpy#189
@rougier

rougier commented Jan 23, 2019

Copy link
Copy Markdown
Member

Good point. Maybe you could also add the color option (as for window clear), what do you think?

@sebftw

sebftw commented Jan 23, 2019

Copy link
Copy Markdown
Contributor Author

That is a good idea.

@sebftw

sebftw commented Jan 23, 2019

Copy link
Copy Markdown
Contributor Author

Should we then add a self._color attribute and color argument to the FrameBuffer class? Like with app.window.Window?

@rougier

rougier commented Jan 24, 2019

Copy link
Copy Markdown
Member

Yes, but there's a risk it messes up with the window clear color.

Removed glClearColor from on_init and the color setter, since it is also set in the clear() function.
This also means that the on_init corresponds to the one in the docstring now.
@sebftw

sebftw commented Feb 7, 2019

Copy link
Copy Markdown
Contributor Author

I thought about it and removed the clear function from the framebuffer, but I now realize it is simple:
The window class clear function always does two things: It sets glClearColor and then calls glClear.
Hence there is no way to mess up the window clear color.

What I did now is that I removed the other calls to glClearColor in the window class. These were present in on_init and color.setter. They are redundant, since glClearColor will be called upon clearing anyways.

Should I just add the framebuffer clear function again?

@rougier

rougier commented Feb 8, 2019

Copy link
Copy Markdown
Member

You're right. Window clear color is protected. For the on_init, we need to make sure glClear is not called elsewhere (I mean, without using the clear function).

For the framebuffer, we could probably do the same and have a dedicated clear function.

@rougier

rougier commented Apr 22, 2019

Copy link
Copy Markdown
Member

I think I forgot about this PR, sorry. What did we decide in the end concerning the clear function?

@sebftw

sebftw commented Apr 22, 2019

Copy link
Copy Markdown
Contributor Author

No problem. I haven't had too much time to look into it anyways.
I think we decided to add a clear function to the FrameBuffer. I am unsure how/if the color attribute should be made for the FrameBuffer. It would be nice if it could be exactly the same as for the Window, but an issue is that the @color.setter is already defined, so we need to make a decision.
A uniform interface for clearing Windows and FrameBuffers or backwards compatibility (no color attribute)?

There are many ways to go about it and I must admit I am too inexperienced to make the call.
It is also possible to just add a setBackgroundColor function to both Window and FrameBuffer, bloating the code somewhat, but preserving backwards compatibility and giving a uniform interface.

@rougier

rougier commented Apr 22, 2019

Copy link
Copy Markdown
Member

I think having a specific clear color for window and framebuffer makes sense. But as you noticed, _color is used in the framebuffer. I proposed to rename _color to _clearcolor that would be also similar to _clearflags in the window.py class. What do you think?

@sebftw

sebftw commented Apr 22, 2019

Copy link
Copy Markdown
Contributor Author

I agree. I'm thinking of adding a @clearcolor.setter to Window and FrameBuffer and just keeping the old @color.setter in them both for backwards compatibility.
Then changing any reference from color to clearcolor in the Window documentation, essentially depreciating it.
I will try implementing it when I get time. That is probably easier than explaining.

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.

2 participants