Windows: Implement Drop for GLContext

kettle11 / kapp

A pure Rust window and input library for Windows, Mac, and Web. (Work in progress)

Apache License 2.0

56 stars 4 forks source link

Windows: Implement Drop for GLContext #59

Closed lunabunn closed 3 years ago

lunabunn commented 3 years ago

Fix #8.

Questions: Where should we DeleteDC? Is it a good idea here to panic in the Drop impl?

lunabunn commented 3 years ago

Didn't mean to do that 😅

kettle11 commented 3 years ago

Looks good!

As for DeleteDC that prompted me to look around a bit and I think the set_window could use improvements:

If None is passed in then ReleaseDC seems like it should still be called, presently it's not.
Also if None is passed in does SetPixelFormat just error?

It also seems like in your Drop implementation that if there's a current window and device context then ReleaseDC should be called.

lunabunn commented 3 years ago

Is there ever a current window but not a DC or vice versa?

kettle11 commented 3 years ago

I was wondering that yesterday, and I don't think so. The Device Context (DC) is associated with a window. They could be coupled together as a tuple within an Option instead of separate options.

lunabunn commented 3 years ago

Here's DeleteDC added to the drop, for now.

kettle11 commented 3 years ago

Cool. Is this ready to merge? We can fix-up the other things I mentioned later.

lunabunn commented 3 years ago

@kettle11 Sorry for the delay. Now it should be ready to merge!