Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

manylinux: Upgrade to SDL2-2.0.22#3475

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
illume merged 1 commit intomainfromsdl-2.0.22-manylinux
Oct 16, 2022
Merged

Conversation

@illume
Copy link
Member

@illumeillume commentedOct 2, 2022
edited
Loading

SDL 2.0.22 release notes are here:https://github.com/libsdl-org/SDL/releases/tag/release-2.0.22

We have windows on 2.0.22 already... so to make it consistent with mac/linux we do this as well.

@illume
Copy link
MemberAuthor

These docker images are pushed.

@ankith26
Copy link
Contributor

Downloaded wheels from CI of this PR, and it's still on SDL 2.0.20 somehow, and it's working well in my basic testing

@illumeillume closed thisOct 3, 2022
@illumeillume reopened thisOct 3, 2022
@illume
Copy link
MemberAuthor

Downloaded wheels from CI of this PR, and it's still on SDL 2.0.20 somehow, and it's working well in my basic testing

Oops. I guess these built before the docker image upload finished. Closed/reopened and I hope this time it works ok.

Copy link
Contributor

@ankith26ankith26 left a comment

Choose a reason for hiding this comment

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

Thankss 🎈 🍕

I openedlibsdl-org/SDL#6333 to track the WSL performance issue I reported earlier.

TL;DR of the issue is that SDL 2.0.22 incorrectly assumes that hardware accelerated texture background is faster on WSL when it's not. A workaround for this issuecould have been settingSDL_HINT_FRAMEBUFFER_ACCELERATION to 0, but unfortunately this hint also broke in the very same commit that introduced the performance regression (but the hint is now fixed on 2.24.0). So if we are going to release this, there are no workarounds that we can provide (that I know of)

Since this bug is not a show stopper for majority of our linux users, I am fine with releasing this with a mention of this issue in the release notes. I am also fine with not releasing this, and waiting for a SDL 2.26.0 where hopefully both this issue and that 2.24.0 windows issue is fixed

Additionally, I also tested an aarch64 wheel of this PR on my raspberry pi 3 and I can confirm that there is no significant changes in performance either way due to this PR, and the examples I tested work fine, and unit tests pass (except the one that#3426 fixes)

Copy link
Contributor

@Starbuck5Starbuck5 left a comment

Choose a reason for hiding this comment

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

If Ankith is fine moving forward with this right now, so am I.

The actual diff in the PR is straightforward to approve.

@ankith26
Copy link
Contributor

Well, I suppose wecould wait for SDL 2.24.2 (on both windows and *nix) since that has a fix for the issue blocking the windows update, and also a workaround for my WSL issue. But if it's too much wait, then we can also release with 2.0.22

@Starbuck5
Copy link
Contributor

Let's go to SDL 2.0.22, having the newer version will probably help other problems that we're not even aware of.

@illume
Copy link
MemberAuthor

Let's go to SDL 2.0.22, having the newer version will probably help other problems that we're not even aware of.

Yeah, agreed. Merging this in, the appveyor failure is unrelated noise.

@illumeillume merged commit465058f intomainOct 16, 2022
@illumeillume deleted the sdl-2.0.22-manylinux branchOctober 16, 2022 11:57
@illumeillume removed the Awaiting MergeFor PRs that have at least one approving review and can be merged on subsequent reviews labelOct 29, 2022
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

2 more reviewers

@Starbuck5Starbuck5Starbuck5 approved these changes

@ankith26ankith26ankith26 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Projects

None yet

Milestone

2.1.3

Development

Successfully merging this pull request may close these issues.

4 participants

@illume@ankith26@Starbuck5

[8]ページ先頭

©2009-2025 Movatter.jp