Skip to content
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

SDL_Texture should be passed as a const pointer #3099

Closed
SDLBugzilla opened this issue Feb 11, 2021 · 0 comments
Closed

SDL_Texture should be passed as a const pointer #3099

SDLBugzilla opened this issue Feb 11, 2021 · 0 comments
Labels
invalid This doesn't seem right

Comments

@SDLBugzilla
Copy link
Collaborator

SDLBugzilla commented Feb 11, 2021

This bug report was migrated from our old Bugzilla tracker.

Reported in version: HG 2.1
Reported for operating system, platform: All, All

Comments on the original bug report:

On 2018-12-12 05:05:36 +0000, Mika Pi wrote:

For example signature for SDL_RenderCopy should look like following:

int SDL_RenderCopy(SDL_Renderer* renderer,
const SDL_Texture* texture,
const SDL_Rect* srcrect,
const SDL_Rect* dstrect);

On 2018-12-14 15:54:53 +0000, Alex Szpakowski wrote:

Those functions may modify some internal state stored in the texture's struct, so the external API can't have a const pointer argument without lying about what it does.

On 2019-05-18 18:20:46 +0000, Ryan C. Gordon wrote:

Those functions may modify some internal state stored in the texture's struct

Yes, and in fact, they do.

--ryan.

On 2019-05-19 16:25:20 +0000, Mika Pi wrote:

If you read from memory, the PC states modifies it's internal state. Bits get moved from DDRAM to the CPU cache. That you said is not an argument. It's may be good to lie in this situation.

On 2019-05-19 17:01:08 +0000, Ryan C. Gordon wrote:

The data pointed to during this function call is modified during this function call. It’s not constant. So we don’t label it as const. This is not a bug.

On 2019-05-19 17:09:59 +0000, Mika Pi wrote:

I did not look in the code but I assume SDL_Texture has some pointer to internal state of GPU or something, and texture from RAM is getting moved to GPU memory, so next time you call SDL_RenderCopy it runs faster? Is it some sort of cache? If it cache it is better to make texture pointer const and internally in SDL_RenderCopy implementation do cast to non-const.

On 2019-05-19 17:18:10 +0000, Alex Szpakowski wrote:

The struct has a pointer to the GPU data, but it also has fields for various other things which are modified during RenderCopy: https://hg.libsdl.org/SDL/file/0280fd2d02ca/src/render/SDL_sysrender.h#l42
(The actual modifications happen both in the high level platform-agnostic RenderCopy code, and in the platform implementations that RenderCopy calls into.)

Since the contents of the struct are modified by RenderCopy, it's not marked as const. If you didn't look at SDL's code beforehand, is there a specific reason for wanting it marked as const other than an assumption about the source code?

On 2019-05-19 17:21:56 +0000, Mika Pi wrote:

Semantically SDL_RenderCopy takes data form the texture and renders in in the renderer, semantically texture is not modified, the function SDL_RenderCopy can do something for performance saike (eg. caching), but for the end user of API it should be invisible.

On 2019-05-19 18:51:57 +0000, Ryan C. Gordon wrote:

(In reply to Anton Te from comment # 7)

Semantically SDL_RenderCopy takes data form the texture and renders in in
the renderer, semantically texture is not modified, the function
SDL_RenderCopy can do something for performance saike (eg. caching), but for
the end user of API it should be invisible.

It’s already opaque data to the API user anyhow, but again, the data is not const, so we will not be marking it—incorrectly—as const.

This is a final answer on this.

@SDLBugzilla SDLBugzilla added bug invalid This doesn't seem right labels Feb 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
invalid This doesn't seem right
Projects
None yet
Development

No branches or pull requests

1 participant