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.

Sign up for GitHub

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

Jump to bottom

enable iOS project and fix compile errors #28

Closed
wants to merge 8 commits into from
Closed

enable iOS project and fix compile errors #28

wants to merge 8 commits into from

Conversation

Longri
Copy link

@Longri Longri commented Jun 19, 2016

No description provided.

@devemux86
Copy link
Collaborator

devemux86 commented Jun 19, 2016 •
edited
Loading

Nice!
Give me some time to check it.

@devemux86
Copy link
Collaborator

devemux86 commented Jun 19, 2016 •
edited
Loading

The vtm-ios/vtm-jni-natives.jar should contain libraries for its platform (iOS) only.
Now it has inside also the Desktop native ones.

@karussell
Copy link

karussell commented Jun 19, 2016 •
edited
Loading

This is interesting! Could be really nice for our routing example. Two questions: isn't robovm dead? Or which fork is this using? (This one?)

@Longri
Copy link
Author

Longri commented Jun 19, 2016

Yes this Fork is the right. It will be preferred by Libgdx (at the moment)

@devemux86
Copy link
Collaborator

@karussell see here for details regarding current libGDX iOS backends.

@Longri
Copy link
Author

Longri commented Jun 20, 2016

But I think it will change in the next time !?
A other fork work on a debugable version! At the moment we have no line numbers at stack trace!

devemux86
devemux86 reviewed Jun 20, 2016
View reviewed changes
vtm-ios/robovm.xml
<lib>../vtm-ext-libs/ios/libgdx.a</lib>
<lib>../vtm-ext-libs/ios/libObjectAL.a</lib>
<lib>../vtm-ext-libs/ios/libvtm-jni.a</lib>
<lib>z</lib>
Copy link
Collaborator

Choose a reason for hiding this comment

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

@Longri what about the <lib>z</lib> here?

Copy link
Author

Choose a reason for hiding this comment

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

I don’t know! The robovm.xml is new generated with latest Setup App!
https://libgdx.badlogicgames.com/download.html

Copy link
Collaborator

@devemux86 devemux86 Jun 20, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

Ah ok, I thought it may be a typo.

devemux86 added a commit that referenced this pull request Jun 20, 2016
@devemux86
Fix iOS project (improves #28), #29, #20
f6b5d9d
@devemux86
Copy link
Collaborator

devemux86 commented Jun 20, 2016 •
edited
Loading

@Longri I merged the PR in issue_28 branch.

See my commit f6b5d9d there with minor improvements.
I also cleared from ios natives jar the desktop libs.

If all are ok I'll merge it in master too.

@Longri
fix iOS native loading
dd83ba5
@Longri
fix assets
122173b 
compile TessJNI into natives
@devemux86
Copy link
Collaborator

devemux86 commented Jun 21, 2016 •
edited
Loading

  • Why the inclusion of jni module in the PR?
  • We should really find a way to leave shaders & themes in their modules, it's counterproductive now.
  • The natives exist as jar and in libs subfolder?

@Longri
Copy link
Author

Longri commented Jun 21, 2016

Sorry! I must more learn!

I think to push my changes to my Fork! I would by save it for me and now I wold clean up!

And fix the upcomming OutOfMemmoryException!

@devemux86
Copy link
Collaborator

That's ok. And the reason it's recommended to have a local branch per PR and rebase it on master, to avoid the "noise" in commits.
At the end probably a new clean PR with only the changed files could do the job.

And fix the upcomming OutOfMemmoryException!

Is the memory issue related to #2?

@Longri
Copy link
Author

Longri commented Jun 21, 2016

maybe this is the same. I will check this

On iOS we can't debug with breakpoints only with LogPrint!
And with a Exception we have no LineNumbers at stack trace!
Debug on iOS is TRY AND BINGO!

@Longri
Copy link
Author

Longri commented Jun 28, 2016 •
edited
Loading

iOS implementation is running without OOM!

I have rewrite complete graphics implementations with iOS depends Bitmap like Android and Desktop!

@devemux86
Copy link
Collaborator

devemux86 commented Jun 28, 2016 •
edited
Loading

Very good! Have you finished with the work?
PR before merge should reflect only the needed changes (like no jni, etc).

Also is there a solution for the shaders / themes location? It'd be best letting them in the original modules.

@Longri
Copy link
Author

Longri commented Jun 28, 2016

Yes, I'm ready!
The JNI Project I turned off again!

The assets are STILL loaded from a separate folder! That will be my next task. but I would like to use a new Clean Fork.
In iOS, the code is only just compiled and here I'll have to extract the relevant Jar's and copy in to the Build folder! Same with Native libs

@devemux86
Copy link
Collaborator

devemux86 commented Jun 28, 2016 •
edited
Loading

Ok I'll check it.

The preferable way is to merge / clean it in a separate branch, until the assets issue is fixed.
Then we can proceed properly with the master, in order to avoid the history clutter.

@Longri Longri mentioned this pull request Jun 28, 2016
Open
@Longri
Copy link
Author

Longri commented Jun 28, 2016 •
edited
Loading

That's fine for me, but can you merge your changes from master in this branch, so I can work with the latest state?

@devemux86
Copy link
Collaborator

Yes of course, I'll notify when finished.

@devemux86
Copy link
Collaborator

devemux86 commented Jun 28, 2016 •
edited
Loading

@Longri see issue_28 branch if all work as intended.

  • I rebased on latest master and squashed for easy reading of changes.
  • I renamed natives jar to vtm-natives-ios.jar and updated it to include only iOS.
  • iOS natives will be used via a jar or uncompressed in libs? Now they exist in two places.
  • We should remove vtm/libs/ios32/libvtm-jni.a.386 from project root. Natives should reside in their module.
  • We should remove duplicate shaders & themes from iOS folder. Assets should be maintained in their modules (for now I updated them with latest work).

@devemux86
Copy link
Collaborator

I pushed a fix in issue_28 branch regarding IosGraphics initialization.

@Longri
Copy link
Author

Longri commented Jun 28, 2016

  • I rebased on latest master and squashed for easy reading of changes.
  • I renamed natives jar to vtm-natives-ios.jar and updated it to include only iOS.
  • iOS natives will be used via a jar or uncompressed in libs? Now they exist in two places.
  • We should remove vtm/libs/ios32/libvtm-jni.a.386 from project root. Natives should reside in their module.
  • We should remove duplicate shaders & themes from iOS folder. Assets should be maintained in their modules (for now I updated them with latest work).

@Longri
Copy link
Author

Longri commented Jun 28, 2016 •
edited
Loading

I think the link to my master is broken !?

Must I create a new PR?

@devemux86
Copy link
Collaborator

Indeed what happened?
I have not changed anything here, except pushing a new issue_28 branch.
Did you perform something in your fork?

@devemux86
Copy link
Collaborator

Oh I see, you deleted the fork and recreate it to get the latest changes?

@Longri
Copy link
Author

Longri commented Jun 28, 2016

Yes, the next wat I have learned ;-) sorry
Do you need a new PR?

@devemux86
Copy link
Collaborator

devemux86 commented Jun 28, 2016 •
edited
Loading

Let me propose a course of actions for contributing via PR and forks that would help:

  • Create a fork of the repository wanting to contribute
  • Create a work branch (based on master) for every contribution
  • Create a pull request from the work branch
  • At any time rebase (not merge) on updated master the work branch

That way the forked master would remain synchronized with remote master and work branch only should contain the changes (GitHub documentation for PR explains all these in detail).

@devemux86
Copy link
Collaborator

devemux86 commented Jun 28, 2016 •
edited
Loading

I suppose a new PR is needed now for working properly.

BTW I checked your fork. You don't need to modify locally you master (that causes trouble), except updating from original repository. You can submit PR from any branch, like the one you work on.

@devemux86
Copy link
Collaborator

devemux86 commented Jun 28, 2016 •
edited
Loading

Closed in favor of PR #42.

@devemux86 devemux86 closed this Jun 28, 2016
@devemux86 devemux86 added this to the 0.6.0 milestone Jun 28, 2016
@devemux86 devemux86 mentioned this pull request Jun 28, 2016
Closed
@karussell
Copy link

Wow, this looks exciting - thanks @Longri & @devemux86 ! Looking forward to use it in our demo :)

andreynovikov added a commit to andreynovikov/vtm that referenced this pull request Aug 18, 2016
@andreynovikov
Viewport toScreenPoint/fromScreenPoint consistency, fixes mapsforge#28
935e8d9
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
enhancement
Projects
None yet
Milestone
0.6.0
Development

Successfully merging this pull request may close these issues.
3 participants
@Longri @devemux86 @karussell