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

Added example for the Marmalade platform #368

Closed
wants to merge 1 commit into from
Closed

Conversation

gzito
Copy link
Contributor

@gzito gzito commented Oct 8, 2015

Hi, I've ported your amazing library to the Marmalade platform, please consider to get it into your master.

Thank you
Giovanni Zito

@ocornut
Copy link
Owner

ocornut commented Oct 13, 2015

Giovanni,
Thanks for this. Some feedback:

  • Are the data/app.icf and marmalade_example.mkb files strictly necessary for such a simple app? (Double-checking. I don't even know that .mkb is)
  • ImGui_Marmalade_GetClipboardText() could first call s3eClipboardGetText() with a NULL pointer to obtain the size and locally store that in a static before returning it? To remove the 512 bytes limit.
  • in ImGui_Marmalade_CharCallback() the test for io.WantTextInput is unnecessary and probably incorrect. We will want text input for shortcuts without setting this flag which is meant to open OSD keyboard.
  • What does Marmelade compile with, and why adding the test for _S3E_ ?
    #if defined(_MSC_VER) && !defined(__S3E__)
    Curious about the reasoning for adding those extra tests.

Thanks!

@gzito
Copy link
Contributor Author

gzito commented Oct 13, 2015

Hi Omar,

  • mkb is a sort of "makefile" for Marmalade; app.icf is the configuration file for the app (e.g. it defines memory needed by the app to run or app orientation) so the're both necessary.
  • Passing NULL as buffer will do the job and will return the size, but then you should dynamically allocate memory for get the text in the clipboard and handle deallocation the next time the callback is called. Is what you meant or have I misunderstood?
  • Ok. I've a question here: how to get text input (for shortcuts) if the OSD keyboard isn't shown?
  • _snprintf() is Microsoft extension to the standard snprintf(), it isn't available in the Mamalade CPP runtime. __S3E__ is defined when you compile for the Marmalade platform, regardless on what compiler you are using (MSVC, GCC or other compiler). So #pragma warning is ok, #define isn't. BTW it's correct - #if !defined(__S3E__) is not necessary in imgui_internal.h :)

I tested the app on my samsung galaxy s5 and the resolution is too high (1920x1080 pixels) so widgets are nearly impossible to tap! How can I scale the UI?

If you agree I can update to the code, test it and commit again before you merge the code. Please let me know. Thanks!

@ocornut
Copy link
Owner

ocornut commented Oct 13, 2015

  • Ok.
  • Anything that would lift the clipboard notification would be ok. Since clipboard action are usually initiated interactively by the user the number of allocations can't sky-rocket. Or at least leave a comment in the code/readme saying there is a clipboard size limitation that can be lifted.
    Clipboard is useful with the log api to copy the content of entire windows (like a tree or a bunch of log items).
  • My intuition is that any device requiring an OSD keyboard won't have access to shortcuts, it wouldn't make much sense. Regardless of this the test as is isn't correct. By the time we add shortcuts or other inputs of character input (holding alt / ctrl) we can probably figure out if we want a user setting to specify when OSD inpiut should be requested.
  • So Marmelade uses MSVC but without _snprintf or its standard library accessible? That sounds actually quite weird. Would a '#define _snprintf snprintf' in your mkb file work? Sorry to be picky on that one but I'd prefer avoiding framework peculiarities to be leaking into the main code.
  • White you're here if you can change from tab 8 to space 4 else i'll do that myself after merging. :)

Thanks again!

@gzito
Copy link
Contributor Author

gzito commented Oct 13, 2015

Ok, I'll remove the clipboard size limitation handling dynamic memory allocation and also io.WantTextInput from the keyboard callback.

Adding #define _snprintf snprintf in the mkb does work, so the main code will be cleaned as before the change (after that I'll commit again and make a new pull request). Thanks.

Also I'll try do my best changing tabs to space.

What about the correct way to "inform" ImGUI that I'm going to scale the UI ? Is there any member variable to set ?

@ocornut
Copy link
Owner

ocornut commented Oct 13, 2015

Sorry I missed on the scaling, there's no direct global scale, the simplest general way is to scale the font as you add it, and things will follow. It probably needs a global scale.

@gzito gzito closed this Oct 14, 2015
ocornut added a commit that referenced this pull request Jul 30, 2016
…les (#323)

Missing support Vulkan (#549), Apple (#575, #247), SDL (#58, #356),
Allegro, Marmalade (#368, #375)
ocornut added a commit that referenced this pull request Sep 18, 2017
…les (#323)

Missing support Vulkan (#549), Apple (#575, #247), SDL (#58, #356),
Allegro, Marmalade (#368, #375)
ocornut added a commit that referenced this pull request Dec 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants