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

OpenPandora port, with separate Commit for GLES, ARM, and Pandora specific #281

Open
wants to merge 25 commits into
base: master
Choose a base branch
from

Conversation

ptitSeb
Copy link

@ptitSeb ptitSeb commented Oct 12, 2013

Now pull request, as I delete the original branch from which the previous pull was based.

Probably not the normal way to do it.

@ptitSeb
Copy link
Author

ptitSeb commented Oct 14, 2013

Sorry about the new Pull request, I delete my old branch, so I guess the old Pull closed itself.

@chino
Copy link
Member

chino commented Oct 20, 2013

That's fine. I have been really busy with the Emscripten port right now and the work you did with EGL/GLES might help there too.. I have the immediate mode (gl1.c) renderer working (https://twitter.com/danielaquino/status/391786141491281920) but it's wicked slow so I'm working on supporting the GLSL backend but since WebGL is based on OpenGLES then I need to get around not having glMapBuffer and other things.

I hope you don't mind using your fork for Pandora builds for a while cause it might take some time to restructure the the various opengl implementations and the changes we're both making so they cleanly implement each backend.

It will probably come down to me merging in a new GLES backend that you'll be able to merge into your branch I hope.

For things like the commit, "debug: remove of some leftover printf" you can simply use git rebase -i then move that commit right before the one that introduced those changes and update the first column from pick to fixup and it will be merged into that previous commit! That will effectively undo those diffs without having to leave them in the history and without having to create a separate commit to remove them.. I'm also working on Emscripten in a branch that is 50 commits ahead of master with a ton of temporary stuff that I will rebase out before I merge back in. You can see my local commits in this screenshot (http://screencast.com/t/EmqrZfIVvMf) which shows I've made quiet a mess out of that branch as I go along but that's ok because I use git rebase -i to squash and remove temporary commits and update commit messages and what not.. That's what makes git awesome because you can keep tracking your history with temporary code / messages and then fix it up before pushing! You also don't have to worry about losing any history since git rebase -i creates a temporary copy of your branch to work on and if anything ever goes wrong you can easily just git rebase --abort to go back to the start! And even if you commit the rebase you can always go back by just changing head to your previous commit id using, git reset $id which you can either lookup that id by checking here or using git reflog which is a history of what HEAD pointed to, so you can instead do git reset HEAD@{10} to reset your local branch back to that commit :] ! Remember when you change history that you already pushed then you need to most likely git push --force to get around the warning that stops you..

@chino
Copy link
Member

chino commented Oct 28, 2013

Have you had a chance to try out rebase-i ? I think your really going to like it..

It seems that this is targeting GLES 1 ?

Do all versions of Pandora support GLES 2 ?

@ptitSeb
Copy link
Author

ptitSeb commented Oct 28, 2013

I haven't tried rebase -i yet.

And yes, my port are targeting mainly GLES1 (because it's what I'm
confident with for now, still learning all this GL stuff), but all Pandora
support GLES 2 too. You have build the OpenGL2 renderer to be compatible
with GLES2?

2013/10/28 Daniel Aquino [email protected]

Have you had a chance to try out rebase-i ? I think your really going to
like it..

It seems that this is targeting GLES 1 ?

Do all versions of Pandora support GLES 2 ?


Reply to this email directly or view it on GitHubhttps://github.com//pull/281#issuecomment-27261829
.

@chino
Copy link
Member

chino commented Oct 29, 2013

Yes actually since webgl is ES2 compatible I had to update my Emscripten
branch to fall back to ES2 calls where required in gl2.c

All I really had to do was use glBufferSubData in place of glMapBuffer and
glDrawElements instead of glDrawElementsBaseVertex.

All you have to do is build with GL=2 to if you wanted to try that out.

That might give you a performance boost if that's an issue since it really
helped for me.

@chino
Copy link
Member

chino commented Dec 31, 2015

Note, for anyone who ends up seeing this later.

The fact that this is still here isn't really an indication that the project is dead.

The actual proper fix for this requires a bunch of code refactoring that no one has attempted to do yet.

A proper fix will apply to this request, Emscripten, and any other platforms that require proper memory alignment.

This branch is functional in the meantime and we could even generate builds from it at some point if people wanted.

It's just a matter of actually wanting to put in the work...

@chino
Copy link
Member

chino commented Mar 27, 2017

Could you add libs back to git ignore?

There is a bunch of random white space changes like deleting empty lines that shouldn't really be in there.

I'd rename ARM define to something more generic like MEMORY_ALIGNED since it applies to more platforms.

I'd like to break this up into separate pull requests to separate RPI, Pandora, memory alignment, opengl changes.

Then we can focus on accepting each one quicker.

The memory alignment fix could be turned into a new routine to rewrite all the file loading code but at this point it's clear it's too much work and might as well get the changes in as are.

@ptitSeb
Copy link
Author

ptitSeb commented Mar 27, 2017

Ok, I'll see what I can do.
I'll start up with cleaning / renaming the stuffs.

I'll see the breakout later.

@chino
Copy link
Member

chino commented Mar 27, 2017

I can help to rebase it all into new pull requests and clean things up if you need me too.

At first site part of the reason I'd like Pandora changes split out is that adding tons of ifdefs all over the code seems like a headache down the road to maintain. I'd like to instead perhaps expose those settings either as a external config file or grouped up into one set of defines. We can do that after it's all split.

@chino
Copy link
Member

chino commented Mar 27, 2017

Also rebasing is your friend here to cleanup commits.

I can do it for you but it might change the author of the commit to me which I'd rather not do.

Copy link

sonarcloud bot commented Jul 18, 2024

Please retry analysis of this Pull-Request directly on SonarCloud

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.

2 participants