-
Notifications
You must be signed in to change notification settings - Fork 464
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
add ClippingRect #1450
base: master
Are you sure you want to change the base?
add ClippingRect #1450
Conversation
src/sdl2/render.rs
Outdated
pub fn clip_rect(&self) -> ClippingRect { | ||
let clip_enabled = unsafe { sys::SDL_RenderIsClipEnabled(self.context.raw) }; | ||
|
||
if let sys::SDL_bool::SDL_FALSE = clip_enabled { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
binding is unnecessary here, just do if sys::SDL_bool::SDL_FALSE == clip_enabled {
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fails to compile
addressed: 76a697e
Definitely needs a changelog entry because it breaks the API. |
@Cobrand there is a changelog entry in changelog.md. Am I missing somewhere else to put it? |
in rust-sdl2, Rect can't have a zero width or height. This is instead represented with Option, where None covers this case.
The Canvas
set_clip_rect
andclip_rect
function work with Option as well. However, for those functions None instead represents the lack of a clipping rect. This is distinctly differently, as clearing the clipping rect is like setting a Rect with an infinite area, and not a zero area (which would instead disable drawing).This is value aliasing which can be corrected with a better type representation; it's impossible with current API to set a clipping rect with a zero area, which is a valid request that disables drawing. Since this has been fixed in sdl2, this MR keep in sync with upstream.
This is not backwards compatible. Thoughts?