Skip to content

Conversation

@HecaiYuan
Copy link
Contributor

We have optimized the functions BlitRGBtoRGBPixelAlphaLSX and SDL_FillRect4LSX opt. @slouken

@madebr
Copy link
Contributor

madebr commented Oct 28, 2025

Feature development happens to SDL3 in the main branch, which then can be back ported.
So can you please also create an equivalent SDL3 PR?

@HecaiYuan
Copy link
Contributor Author

Okay. No problem. @madebr

@slouken
Copy link
Collaborator

slouken commented Oct 28, 2025

Offhand, these look good. Do you have performance numbers that show the difference between LSX and non-LSX code paths for these operations?

@slouken slouken added this to the 2.x milestone Oct 28, 2025
@jinboson
Copy link

jinboson commented Oct 30, 2025

Hi @slouken ,we didn't find a good way to show the performance. We wrote these optimizations based on perf record results when using SDL to render videos. After these optimizations, and together with other optimizations merged before(see 50d8642), we find that the video renderings become more smoothly.

@HecaiYuan
Copy link
Contributor Author

@madebr. an equivalent SDL3 PR has been created. #14385

@jinboson
Copy link

jinboson commented Nov 4, 2025

Hello, is there anything else that needs updating?

@slouken
Copy link
Collaborator

slouken commented Nov 4, 2025

Hello, is there anything else that needs updating?

Well, this PR doesn’t have -mlsx of the SDL3 PR, so I assumed it wasn’t ready yet. Was that not necessary, or is it already added elsewhere?

The renderer change was also not present in the SDL3 PR, so I assumed it wasn’t necessary in this one either.

Android_ActivityMutex_Lock_Running();
#endif

/*CPU acceleration runs faster than integrated graphichs on Loongson at pressent.*/
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/*CPU acceleration runs faster than integrated graphichs on Loongson at pressent.*/
/* CPU acceleration runs faster than integrated graphics on Loongson at present. */

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

Comment on lines +1005 to +1010
#if defined(__linux__) && defined(__loongarch__)
if (SDL_CheckIntegratedGraphics()) {
SDL_SetHint(SDL_HINT_FRAMEBUFFER_ACCELERATION, "0");
flags = SDL_RENDERER_SOFTWARE;
}
#endif
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will effectively make it impossible to create an opengl SDL_Renderer on those platforms. Is this ok for compatibility with non-SDL renderer using OpenGL?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @madebr, what does it mean with non-SDL renderer using OpenGL?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With non-SDL renderer using OpenGL, I mean a user project using OpenGL calls for drawing (including 3D) instead of using the SDL renderer.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your quick reply, it seems this change will not affect that behavior.

@jinboson
Copy link

jinboson commented Nov 5, 2025

Hello, is there anything else that needs updating?

Well, this PR doesn’t have -mlsx of the SDL3 PR, so I assumed it wasn’t ready yet. Was that not necessary, or is it already added elsewhere?

The renderer change was also not present in the SDL3 PR, so I assumed it wasn’t necessary in this one either.

Sorry, they are really missed. Will add them lately.

@jinboson
Copy link

jinboson commented Nov 5, 2025

Hi @slouken , I misunderstood your question about -mlsx option, which is not required in this PR, since it has been added globally in 17f63e5. The renderer change was not present in SDL3 PR, that's because the API of SDL_CreateRender has changed there, we are considering how to rewrite the implementation or whether the change is needed still.

BTW, this PR is ready for review.

@slouken
Copy link
Collaborator

slouken commented Nov 5, 2025

Hi @slouken , I misunderstood your question about -mlsx option, which is not required in this PR, since it has been added globally in 17f63e5.

Should it be added globally in SDL main?

@slouken
Copy link
Collaborator

slouken commented Nov 5, 2025

Please remove the renderer change from this PR. You can submit that separately for consideration.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants