Skip to content

HarmonyOS/OpenHarmony Port #13152

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

Open
wants to merge 70 commits into
base: main
Choose a base branch
from
Open

Conversation

Jack253-png
Copy link

Description

SDL support for harmonyos

Existing Issue(s)

#9837

@Jack253-png
Copy link
Author

@slouken Request for review

@Jack253-png
Copy link
Author

@madebr @sezero Request for review

@madebr
Copy link
Contributor

madebr commented May 31, 2025

Can you please rebase on top of current master and remove all commits which you did not author?

git fetch https://github.com/libsdl-org/SDL main
git rebase -i FETCH_HEAD
<remove all commits which you did not author. In vim, "dd" removes the current line, "<ESC>:wq" saves and exits)>
git push [email protected]:Jack253-png/SDL HEAD:OpenMinecraft-Dev -f

@madebr
Copy link
Contributor

madebr commented May 31, 2025

Seeing you implemented vulkan support, does SDL_gpu work? Do the SDL_gpu_examples work?

@Jack253-png
Copy link
Author

Seeing you implemented vulkan support, does SDL_gpu work? Do the SDL_gpu_examples work?

I will test later, because I don't have test platform, I need to find other people to do this.

@Jack253-png
Copy link
Author

Does it work? I changed the commit message

@sezero
Copy link
Contributor

sezero commented Jun 8, 2025

Does it work? I changed the commit message

Hmph. It still seems to run all the workflows. @madebr: Is the filtering correct?

@madebr
Copy link
Contributor

madebr commented Jun 8, 2025

Does it work? I changed the commit message

Hmph. It still seems to run all the workflows. @madebr: Is the filtering correct?

I'm using ${{ github.event.head_commit.message }} to get the commit message.
It looks like this works for the run in Jack's fork, but does not for this PR.

@Jack253-png
Copy link
Author

Does it work? I changed the commit message

Hmph. It still seems to run all the workflows. @madebr: Is the filtering correct?

I'm using ${{ github.event.head_commit.message }} to get the commit message.
It looks like this works for the run in Jack's fork, but does not for this PR.

Do you find the problem? Why it works in my repo?

@madebr
Copy link
Contributor

madebr commented Jun 8, 2025

On pushes, github.event is a push object, which has a .head_commit.message` property.

On pull requests, github.event is a pull_request (synchronize) object. This objects does not have .head_commit.message property. Through github.event.pull_request.commits_url, we can find out the commit message.
This url returns paged data with a maximum of 100 items. We need the last item, so it'd require to iterate the list.
The event has github.event.pull_request.merge_commit_sha, but does not return the sha of the head commit of the pr. Doing something like git log -n -1 HEAD~1 does not reliably return the correct commit (remember, we test merge commits in pull requests, not the actual head commit)

@Jack253-png
Copy link
Author

On pushes, github.event is a push object, which has a .head_commit.message` property.

On pull requests, github.event is a pull_request (synchronize) object. This objects does not have .head_commit.message property. Through github.event.pull_request.commits_url, we can find out the commit message.
This url returns paged data with a maximum of 100 items. We need the last item, so it'd require to iterate the list.
The event has github.event.pull_request.merge_commit_sha, but does not return the sha of the head commit of the pr. Doing something like git log -n -1 HEAD~1 does not reliably return the correct commit (remember, we test merge commits in pull requests, not the actual head commit)

It is an existing bug in SDL GitHub workflow?!

@madebr
Copy link
Contributor

madebr commented Jun 8, 2025

It is an existing bug in SDL GitHub workflow?!

Yes and no.
Yes because parsing commit messages do not work.
No because pull requests creators cannot sneakily disable a workflow.

So yes, the problem was already present.

@sezero
Copy link
Contributor

sezero commented Jun 8, 2025

. No because pull requests creators cannot sneakily disable a workflow.

So yes, the problem was already present.

Actually that's a good thing and not a problem. A patch causing problems
elsewhere would go undetected otherwise.

@Jack253-png
Copy link
Author

@icculus @sezero opengles2 tested, works (but the renderer doesn't work due to the shader problem, I wrote a demo by myself), after solving the multi threading problem, I will put the ohos project into this pr. It is near to the merge

@Jack253-png
Copy link
Author

@madebr @sezero request for review

@Jack253-png
Copy link
Author

@madebr review

@madebr
Copy link
Contributor

madebr commented Jul 1, 2025

This looks very great. I've got some questions.

  • This adds support for OpenHarmony 5. What compatibility promise does OpenHarmony make? Do apps built for older SDK's run on newer versions?
  • Can you document how to package a SDL3 app and create an OpenHarmony app?

@Jack253-png
Copy link
Author

This looks very great. I've got some questions.

  • This adds support for OpenHarmony 5. What compatibility promise does OpenHarmony make? Do apps built for older SDK's run on newer versions?
  • Can you document how to package a SDL3 app and create an OpenHarmony app?

For now the port works on any OpenHarmony/HarmonyOS 5.x/6.x systems (I have tested in the emulators), 4.x (older versions is end-of-life)

Now the building of the app requires DevEco Studio, which is very hard to acquire except in China, but I am trying to put the sdl-ohos-shell project into this repo, and I planned to write the document after the audio subsystem porting

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