Renderer.fill_rect now takes an Option<Rect>.

[Zoomulator]

Native SDL2 behavior allows passing of null to fill the entire target.

Unless there's some reasoning not to allow this?

[Cobrand]

Luckily for us Into<Option<P>> has been implemented for P few stables ago, so this change shouldn't break anything, but we can never be too sure. It's not that important if it did break anything cause technically we are unstable, but it'd be nice to know ourselves if it breaks or not.

Technically I guess renderer.clear() does the same thing but if there are 2 functions I guess they are actually different ? Do you know the difference between clear() and fill_rect(None) ?

If you could confirm that this change doesn't break current usage of renderer.fill_rect(rect), I'll gladly merge this. Even if it does, we'll just break the API and bump to v0.26

[Zoomulator]

The difference is that renderer.clear() ignores blend mode and can not fill with a transparent color, while doing the following does:

renderer.set_blend_mode( BlendMode::Blend );
renderer.set_draw_color( Color::RGBA(0,255,0,100) );
renderer.fill_rect( None );

There's no implicit conversion from Rect to Option<Rect> as far as I can tell. From::from(rect) works fine. Not sure if you meant Into<Option<P>> possibly would allow implicit conversion?

So I'd imagine this is indeed a breaking change.

[Zoomulator]

Hold on, I got you now. I'll implement the fill_rect function taking an T: Into<Option<Rect>>. Should be implicit then.

[Cobrand]

Nah, I mean that few rust releases ago this PR has made its way into the language : rust-lang/rust#34828

See here

That means that you should not have to implement anything, it is already in the language. However I'd like to know if this really works or not right now with our code (it should be, but we can never be too sure)

[Cobrand]

Sorry technically you just have to change one line, which is

pub fn fill_rect(&mut self, rect: Option<Rect>) -> Result<(), String> {

into

pub fn fill_rect<T:Into<Option<Rect>>(&mut self, rect: T) -> Result<(), String> {
    let rect = rect.into()

edit: just realized that's exactly what you said, so this is useless

[Zoomulator]

Yep, that's exactly what I just realized. :) Thanks anyway for the clarification.
I've cleaned up the branch so there's only one commit,

[Cobrand]

Cheers !