Add Renderer.logical_to_window and Renderer.window_to_logical from SDL2

[DickerDackel]

See https://wiki.libsdl.org/SDL2/SDL_RenderLogicalToWindow and https://wiki.libsdl.org/SDL2/SDL_RenderWindowToLogical

When setting renderer.logical_size, these two functions offer translation between window space and logical space when the renderer is scaled different from the window.

This is e.g. useful to translate mouse coordinates that are in window space, to logical_size coordinates of the renderer (or back).

[DickerDackel]

I've seen the failing stub stuff, have no experience with it, but just had a look at the github action. Fix coming...

[ankith26]

On the SDL3 porting guide I found this note

SDL_RenderWindowToLogical() and SDL_RenderLogicalToWindow() have been renamed SDL_RenderCoordinatesFromWindow() and SDL_RenderCoordinatesToWindow() and take floating point coordinates in both directions.

I think we should consider whether we should stick to SDL2 terminology or use SDL3's. In either case, I think it makes more sense to keep input consistent for both methods (a single Point argument, like rest of the functions in this module) and a tuple[float, float] return type.

[DickerDackel]

I think we should consider whether we should stick to SDL2 terminology or use SDL3's.

I've not yet been in this situation, so this is unfounded gut feeling, but if I was using the _sdl2 API, I'd look up the SDL2 docs.

How about aliasing them to provide both names? Like Vector2's "lengh" and "magnitude" methods?

In either case, I think it makes more sense to keep input consistent for both methods (a single Point argument, like rest of the functions in this module) and a tuple[float, float] return type.

Sure, I'll change that. Give me an hour or so...

[DickerDackel]

like rest of the functions in this module) and a tuple[float, float] return type.

Looking at the other functions, e.g. draw_line, draw_point, they also all accept kwargs, which brings us to naming conventions. draw_point has a point argument, draw_line accepts p1 and p2. I'm an old guy, so I'm happy with p, will assume people would prefere the more obvious point, but will (probably) also happily implement any other suggestion.

Leave any thoughts here, I'll go with point until I hear something else, since that's pretty much what we do translate.

[ankith26]

LGTM, thanks for the PR! 🎉

But before we merge it, we should also consider whether the same functionality must be implemented in pygame._sdl2.video which is the original implementation. I haven't been keeping up with pygame._render so I don't know what the plan is, requesting @Starbuck5 and/or @MightyJosip to review this aspect.

[damusss]

I haven't been keeping up with pygame._render so I don't know what the plan is

can't assure my info is correct, but _render is a C implementation of sdl2_video. so from what I know _render (future render) is the "official", future-to-be public implementation of sdl2_video. it's like pygame.window and sdl2_video.Window, the latter (second one, I always confuse former and latter) stopped receiving updates, and in my opinion it makes sense to keep updates to _render.

[damusss]

Everything looks good to me and makes sense. not merging right away to allow more contributors to check it since it would be new api (tho not immediately public)

[MightyJosip]

LGTM. But we need to consider what to do with the docs. Because by the way you currently did it, this functionality would be available in pygame._sdl2.video.Renderer as well, however, it exists only in pygame._render. After the testing phase, it should probably be moved to another module docs, like it currently works for Window https://pyga.me/docs/ref/sdl2_video.html#pygame._sdl2.video.Window

[DickerDackel]

I followed the existing doc strings in render.c. I just also grepped the whole project directory for one of the existing doc strings, and there is no other place right now where these are referenced.

So I see three options:

1. As discussed above, create these two functions in the cython sdl2_video too.
2. create a dedicated doc include for render, but that would go against what Damus did here: d1f5a4b
3. Leave it as is, until the render module is official and then move all DOC_SDL2_VIDEO_RENDERER_* back into a dedicated doc file.