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

iOS example #247

Closed
wants to merge 10 commits into from
Closed

iOS example #247

wants to merge 10 commits into from

Conversation

joeld42
Copy link
Contributor

@joeld42 joeld42 commented Jun 20, 2015

This is the iOS default Xcode "OpenGL" template (which draws two spinning cubes), modified to support iOS and uSynergy. More details in the README.md in the ios_example directory. It's based mainly on the glfw and opengl3 examples. Tested on iOS8 on iPad and iPhone (iPhone works but the screen gets reaallly crowded).

@ocornut
Copy link
Owner

ocornut commented Jun 20, 2015

Thanks!

Just skimming through this very briefly

  • why are there shaders in the code in imgui_impl_ios.mm and also two separate files? there's also calls to e.g. glCompilerShader() in both GameViewControllers.h and imgui_impl_ios.mm (which says imgui_impl_ios.cpp in the header)
  • I would move uSynergy.cpp/uSynergy.h to libs/synergy/ outside the ios folder, seeing it can be useful to other implementations and to emphase that this is separate code.
  • It looks like synergy keyboard mapping is hard-coded which wouldn't work for all sort of keyboard (I did the same thing last time I used synergy a few years ago). I wonder if it's just a missing feature in the Synergy API that "character inputs" are not passed or if it's missing from uSynergy. It would make more sense that the remote system converts keys to characters, Be worth investigating this..
  • i would prefer if the structure of the application mimiced the other samples more closely but I don't really understand the relationship between Obj-C and C++ at this point so it may be just fine.
  • are the icons strictly necessary? it's cool there's an icon but having a dozen file

I don't have a Mac here to test and I'd probably just merge it untested but it would be nice to look at least the 2 first points

@joeld42
Copy link
Contributor Author

joeld42 commented Jun 20, 2015

All good points...

The shaders in imgui_impl_ios.mm are the imgui ones, all of the shader code in the .vsh/.fsh are from the Xcode opengl example, they are used for drawing the cube. I was trying to leave the xcode example as untouched as possible.

I will move the usynergy code, I was just going for self-contained.

The keyboard mapping is a big headache. I tried for a while to find a better way to do this, but this was the best I could come up with. There are some device independent API's available to convert scan codes to unicode characters, but these are only available on mac and not on iOS as far as I can tell (it's part of Carbon). I didn't see any better way to do this or any way to get the character codes out of usynergy. Hardcoding these was enough for my purposes but I can ask on the uSynergy forums, maybe there's a better way. If I find a better way, I'll be happy to implement it. I added a comment with a note about this at least.

I also added a note in the example Readme, and in the connect func reminding that you need to turn of Synergy's SSL mode since usynergy does not support SSL.

As for the icons, that's just the way iOS apps are, it's pretty annoying. I have a script that just makes all the sizes I need. I removed the optional ones (spotlight and settings icons), but there's still 4 required icon sizes.

@ocornut
Copy link
Owner

ocornut commented Jun 22, 2015

Looks like current synergy can't send character but older versions where able to. Searching for "character" in their issues tracking shows a lot of issues related to keyboard inputs because it appears to requires the client to translate keycodes into characters.
I added a comment here:
https://github.com/synergy/synergy/issues/4644

It looks like we could keep the iOS example like that at the moment but it would be useful for push for Synergy to get the fix (I am having the same issue with another project running on consoles). Won't be able to access a mac for testing for a while probably :/

@ocornut
Copy link
Owner

ocornut commented Jul 8, 2015

OK i have merged that in. I have rebased it into one commit to lose the duplicate PNG and applied a few tweaks, typos and code formatting.

I HAVEN'T TESTED IT. This is the first time I don't test something in master but since it doesn't touch the main library files it is acceptable. When I get around to get it running I will look into Synergy integration, etc. Would be best if you could get master back and see if I haven't broken anything.

@ocornut
Copy link
Owner

ocornut commented Jul 8, 2015

@joeld42 Would you be kind to check out the Anti-Aliased branch and fix the iOS example accordingly? The code has switched to indexed rendering and I have done a little renaming but it should be quite easy. Refer to the "API breaking changes" section of imgui.cpp
Thanks a lot!

@joeld42
Copy link
Contributor Author

joeld42 commented Jul 8, 2015

Thanks for merging this! I tried this out in master, and it works fine for me. I'll try and get some other folks to try building it, in case there's something that depends on my configuration.

I fixed the ios example on the anti-alias branch as well, sent you a separate pull request for that.

@ocornut
Copy link
Owner

ocornut commented Jul 15, 2015

AA branch is merged in now, and I've made various changes to the font system to support subpixel positioning along with oversampling (not enabled for the default font since it doesn't need/want either). But may be useful for smooth fonts on iPad.

@ocornut ocornut closed this Jul 15, 2015
@ocornut
Copy link
Owner

ocornut commented Jul 19, 2015

Made change to the OpenGL3 example which I believe could be applied to the iOS example.
Basically we are just using:

for (int n = 0; n < draw_data->CmdListsCount; n++)
{
    const ImDrawList* cmd_list = draw_data->CmdLists[n];
    const ImDrawIdx* idx_buffer_offset = 0;

    glBindBuffer(GL_ARRAY_BUFFER, g_VboHandle);
    glBufferData(GL_ARRAY_BUFFER, (GLsizeiptr)cmd_list->VtxBuffer.size() * sizeof(ImDrawVert), (GLvoid*)&cmd_list->VtxBuffer.front(), GL_STREAM_DRAW);

    glBindBuffer(GL_ELEMENT_ARRAY_BUFFER, g_ElementsHandle);
    glBufferData(GL_ELEMENT_ARRAY_BUFFER, (GLsizeiptr)cmd_list->IdxBuffer.size() * sizeof(ImDrawIdx), (GLvoid*)&cmd_list->IdxBuffer.front(), GL_STREAM_DRAW);

    for (const ImDrawCmd* pcmd = cmd_list->CmdBuffer.begin(); pcmd != cmd_list->CmdBuffer.end(); pcmd++)
    {
        if (pcmd->UserCallback)
        {
            pcmd->UserCallback(cmd_list, pcmd);
        }
        else
        {
            glBindTexture(GL_TEXTURE_2D, (GLuint)(intptr_t)pcmd->TextureId);
            glScissor((int)pcmd->ClipRect.x, (int)(height - pcmd->ClipRect.w), (int)(pcmd->ClipRect.z - pcmd->ClipRect.x), (int)(pcmd->ClipRect.w - pcmd->ClipRect.y));
            glDrawElements(GL_TRIANGLES, (GLsizei)pcmd->ElemCount, GL_UNSIGNED_SHORT, idx_buffer_offset);
        }
        idx_buffer_offset += pcmd->ElemCount;
    }
}

To set the buffers. Removing g_VboSize, adding a handle for the element buffer g_ElementsHandle.
glDrawElements() when an element array buffer is set takes an offset, and the buffer is validated and uploaded at the time of calling glBufferData()

See #278

I imagine that would be more standard/compatible and shorter. If you have time could you take a look at applying the same patchs? Thanks :)

ocornut added a commit that referenced this pull request Jul 21, 2015
@ocornut ocornut mentioned this pull request Apr 5, 2016
ocornut added a commit that referenced this pull request Apr 5, 2016
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)
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