Raspberry Pi support #2837
Raspberry Pi support #2837
Conversation
|
@hinxx would you mind to test the cmakeified version from https://github.com/NeroBurner/imgui/tree/example_sdl_vulkan_cmake of |
|
@NeroBurner here are the results: SDL1 w/ openGL2: SDL1 w/ openGL3: |
| ##--------------------------------------------------------------------- | ||
| ## OPENGL LOADER | ||
| ##--------------------------------------------------------------------- | ||
|
|
||
| ifndef RPI |
There was a problem hiding this comment.
Currently if new platform flag was introduced this bit will have to be rewritten as it wont really make sense any more. Instead if we had ifdef RPI here we could just append a case of new platform in the future without altering old code.
There was a problem hiding this comment.
Sounds like a sane comment, looking into future.
|
|
||
| ifndef RPI |
There was a problem hiding this comment.
I am not aware of Makefile conventions, but indentation here does look confusing. Do people treat ifdef/endif in Makefiles as #ifdef or as if in c?
There was a problem hiding this comment.
As far as I know, Makefile syntax in this case does not impose the rules about indents (just like C does not). IMO, it is more a matter of convention and what original code uses.
I used indent as they were used before my change. I should have followed this approach for the ifndef on lines 32 and 40 as well to be consistent.
| #if defined(IMGUI_IMPL_OPENGL_ES2) | ||
| #include <GLES2/gl2.h> | ||
| #elif defined(IMGUI_IMPL_OPENGL_ES3) | ||
| #if (defined(__APPLE__) && (TARGET_OS_IOS || TARGET_OS_TV)) |
There was a problem hiding this comment.
Where do TARGET_OS_IOS and TARGET_OS_TV come from? I do not see them in Makefile. Does platform define them?
There was a problem hiding this comment.
They're in <TargetConditionals.h>. Include is missing.
There was a problem hiding this comment.
TARGET_OS_XX macros are defined by Apple headers, specifically TargetConditionals.h.
What's not absolutely clear is if that file is included even indirectly from main.cpp, maybe including it explicitely would be good (like imgui_impl_opengl3.cpp does)
There was a problem hiding this comment.
But if we can establish that e.g. SDL.h always include it I'm happy relying on that knowledge.
There was a problem hiding this comment.
Seems that compiler / pre-processor define those. Some insight here: http://nadeausoftware.com/articles/2012/01/c_c_tip_how_use_compiler_predefined_macros_detect_operating_system#OSXiOSandDarwin
It would be nice to detect 'platforms' like RPi like this but as it happens, to compiler RPi CPU looks just like any other ARM based CPU. While ARM CPU can be distinguished from x86, it is too wide.
There was a problem hiding this comment.
If __APPLE__ is defined "TargetConditionals.h" is safe to include and I think it should be. This is the place where all TARGET_XXX macros are defined.
If SDL.h include it it would be for very same reason it should be included here. Those are macros and they are leaking into user code, relying on that does sound like a good idea.
This type of target detection isn't unique to Apple platforms. Windows SDK provide <winapifamily.h> serving very same purpose.
There was a problem hiding this comment.
what about grep -o BCM2708 /proc/cpuinfo for RPI detection? (or BCM2709 for the rpi 2 & 3). However this would not work when cross compiling. Perhaps another approach is to check for /opt/vc/include/bcm_host.h
|
Trying to do this on latest Raspbian and latest master gives the error: |
|
Ok, it happens when the linking is wrong. It should link with the GLES libs in /opt/vc not the ones in /usr. It runs fine now |
|
@sphaero my pull request explicitly uses /opt libs. I tried with the ones from apt , that end up in /usr back then and did not get far. |
|
BTW, what is the current status of this pull request? Is there something preventing the merge? |
|
Yes, the ones in /usr are not hardware accelerated and require a Xorg session. If I would need to provide feedback to the PR I would question whether it's not using the |
I tried now with Rpi branch on raspberry 3b+, and got same error! I also checked the libs are linking to /opt/vc and see similar to what you have except I am missing few ones, not sure why Question: Is this working pr now? |
I noticed there were some updates to the files my PR is also touching. I just updated my PR with the latest upstream master code and re-tested on RPi. Works as expected for me. Here are some notes about how I did it: NOTES.txt And the device OS info: |
|
I can test here, tbh, I think most people using the RPi are going to be building custom so I would have no issue with having hardcoded settings for it. |
I guess by 'custom' you mean with the custom folder layout if doing cross compiling? If so then it is pretty much up to the selected tools and placement of the /opt/vc contents on the host. |
|
I tried to cross compile the SDL opengl3 example and seems that one is free to decide where to put headers and libs from I picked the official one at https://github.com/raspberrypi/tools for my tests today. In order to cross compile I took the official OS image and mounted the rootfs part on host PC; this way I could get a hold of the Next I needed to get SDL2 cross compiled for RPi and installed on the host (also required on the RPi if doing native build). Then I got around and tried to revisit the What would be added is a Source I need to cleanup the changes I have and then I'll update the PR. |
|
Hi @hinxx, I just found your PR and was wondering if you'd be open to fix the mouse position detection issue in a slightly different way? I'm referring to this change: 361f68b#diff-62603e3f16b4d34a44a31904aea9bed53b56b01fdba50f9fb84c8c416eeb2e77R235 My suggestion would be that instead of checking for a g_MouseCanUseGlobalState =
strncmp(SDL_GetCurrentVideoDriver(), "wayland", 7) != 0 &&
strncmp(SDL_GetCurrentVideoDriver(), "RPI", 3) != 0;Edit: Thinking about it some more, I decided to check for all known supported backends instead of checking for known not working ones. See lethal-guitar/RigelEngine@597f1b8 With that change, mouse interaction works correctly for me on my Pi 3. The advantage of this approach is that as a user of Dear ImGui, I don't have to define Let me know what you think. I'd also be happy to open a follow-up PR of my own after you've merged yours if you'd prefer to leave your solution as is 🙂 |
|
@lethal-guitar I would need to remind myself of what the PR does, but I'm in favor of any change that would remove RPI define. You said that you do not need a RPI define to build it; I presume this is done natively on the Rpi itself. Seems the other code I wrapped in the RPI define can therefore be used as-is, without the RPI define, or am I missing something here? |
Exactly, in my project, I don't have a RPI define. I only have a define to distinguish between OpenGL ES and Desktop GL. To build for the Pi, I just need to enable the GL ES define, which then defines I should note however that I'm using glad as a loader for OpenGL ES (See here). This has the nice advantage that I don't need to link to any Raspberry Pi specific libraries at build time. I think unless we would do the same in the ImGui examples, we still need some Pi-specific logic in the Makefile, to link against the Broadcom VC stuff.
I think you'd need to test for Other than that, I believe yes, this code should work for any OpenGL ES build, Pi or something else. |
I got openGL3.0 and OpenGL ES 3.1 on RPi 4 (aarch64), (just in case you are interested) : https://framagit.org/ericb/ir_thermography/-/commit/1b11a9f31de14db80156c108f0c874dd87fbe8ae Edit : don't forget RPi3 b+ is armhf, not aarch64 ! |
|
@ocornut your summary of the three topics sounds right to me. The mouse position fix does indeed seem like something you could merge right now. My proposal is to do it like here: lethal-guitar/RigelEngine@597f1b8 I also believe that checking for the GL ES define instead of So |
|
@ocornut I went ahead and opened two PRs, to tackle the first two topics. Let me know what you think. @hinxx @hippyau I noticed that linking to |
|
Wow, lots happened in the last three days on multiple fronts! I'm a bit out of sync what the status of RPi is ATM, and what still remains to be solved by this PR. What remains would be the build script which I volunteered to make a while ago (and never came around to do it). I can look into the script in the coming days, though, in the light of having this PR closed. Is there a fork/branch of the all RPi changes currently agreed upon that I could test and work of? Or should I just pick up master branch here? Thank you and apologizes for being slow! |
|
@hinxx there is still #3951 which I need to do a bit of work on before it can be merged - but once that's done, all changes aside from the build script will be on Regarding the Btw, I've already included the Raspberry Pi specific build flags in the So after that PR is merged, building for RPI should already be possible, but people will need to edit the Regarding cross compilation, one problem I think would be how to specify the location of the |
| CXXFLAGS += `sdl2-config --cflags` | ||
| else | ||
| ECHO_MESSAGE = "Raspberry Pi" | ||
| LIBS += `PKG_CONFIG_PATH=/opt/vc/lib/pkgconfig pkg-config --libs bcm_host brcmegl brcmglesv2` -ldl `sdl2-config --libs` |
There was a problem hiding this comment.
This is not required on a Pi4/3 running with the new OpenGL KMS driver.
Standard pkg-config is just enough.
pkg-config glesv2 --libs
It is required if one really wants the legacy driver.
There was a problem hiding this comment.
I do not know much about the the KMS driver.
Do you have any input on what are differences between the legacy vs. KMS driver?
Does the KMS driver postulate use of X11?
There was a problem hiding this comment.
This is not required on a Pi4/3 running with the new OpenGL KMS driver.
Standard pkg-config is just enough.
pkg-config glesv2 --libsIt is required if one really wants the legacy driver.
I am doing this right now, and getting 60FPS, but seeing 100% usage of all 4 cores. Not really sure why. My earlier pull request does not work on Rpi4 because I am not sure the legacy is even available.
There was a problem hiding this comment.
I am doing this right now
Which Pi? Which KMS? It is a pain, but without this info, one cannot understand.
Anyway, this PR, on a Pi4 using Full KMS, gives * failed to add service - already in use?
IIRC, the Pi4 does not have the legacy driver.
On the other hand, if I do this
LIBS += -lGL -ldl `sdl2-config --libs`
CXXFLAGS += `sdl2-config --cflags`
It works. Maximised (not fullscreen), about 10% CPU, 60 FPS.
There was a problem hiding this comment.
Sorry, was talking about Pi4, no X....
I have just seen the weirdest thing...
running example_sdl_opengl3 gets 100% CPU and ~40 FPS...
running sudo example_sdl_opengl3 gets 60FPS solid and 10% CPU.
|
I have been trying to follow the Pi discussion. But the "standard" case (I know I am a bit arrogant...) of running inside X11 using the new KMS drivers does not need any special handling. The same build script works on Ubuntu 20.10 Haswell OpenGLES2 as well as it does on a Pi3/4, and nowhere one needs to know that it is a Pi. Could it be that for most people this is all they need to know? These are my cmake scripts https://github.com/audetto/AppleWin/blob/master/source/frontends/sdl/CMakeLists.txt#L3 Nowhere I check for a Pi. |
|
Not sure how helpful this can be, but i worked on raspberry support recently. I wrote down everything in the readme: https://github.com/Pesc0/imgui-cmake |
Interesting. On a Pi4, using standard SDL, OpenGLES2 ImGui, inside X11, I get
OpenGLES3 makes no difference. Goes without saying that a smaller window achieves higher FPS. These are the worst cases. Do you know what FPS you can achieve? I ask because I've read all sort of results about FPS on a Pi, and there is not a clear reference to benchmark. |
|
@audetto it's true that nothing special is required when using the KMS drivers. But people might still want to use the legacy drivers when running without X on a Pi 3/Zero or earlier. I don't have concrete numbers at hand right now, but I was definitely getting better performance on a Pi 3 B when using the legacy drivers vs when running inside X. I haven't compared to KMS/DRM without X, though. This might have changed recently, but I also believe that Raspbian Buster still defaults to the legacy driver when running on Pi 3 or older? So the "out of the box" experience you describe would only work if users enable the KMS driver first, or am I misunderatanding something? As I mentioned, it's also possible to set things up in a way that no special build configuration is required regardless of which driver is used (Legacy or KMS), by loading the GL ES library at run-time using a loader that supports GL ES, like |
Frist things first: what im about to say applies to legacy driver, without X (fullscreen 1080p). I couldn't get KMS to work, but havent tried too hard, since for me kms caused some visual glitches even in the raspbian desktop, like the mouse cursor looking like a big square pixelated with random values. |
|
I guess we all have in mind different "standard" cases, and Pi3 != PiZero != Pi4. @Pesc0 You did not say which Pi you use. I can get the colored buttons at 88 FPS fullscreen. The new Full KMS on a Pi4 is a incredible improvement on anything that was existing before. @lethal-guitar You are right and I don't know what the default KMS flag is on a Pi3. I know on a Pi4 it default to Would probably be worth adding which use case is being considered when one simply says "On Pi you need this". |
Yeah it's a good point, agreed! |
|
@audetto Yeah sorry about that. Its a Pi3. I'm going to update my repos readme as well to include this "benchmark" information. |
|
Thanks @lethal-guitar ! I took your branch from #3951, changed the Makefile a bit, compiled on Pi3 revB, and got it running without any other changes to the C++ code. Good job! Makefile changes I made: For the record, I installed stock |
|
In regards to KMS comments above, let me just say what was on my mind at the time when this PR was initially proposed. My goal was to use what was available and considered stable (what today can be called legacy BRCM GLES driver). I did not have Pi4 at that point (as it was released on June '19 at a hefty price tag). AFAICT, KMS was something introduced with Pi4. My goal was (and still is) to make ImGui available on Pi, no matter what hardware release. This is possible even with Pi4 on the table. Legacy GLES driver works on all of them and having ImGui running on all of them is what makes most sense to me. Driver is proven to work, exists for years and has a lot of history, and example code outside SDL, too. Even with the KMS in town, RPi folks have not dumped the legacy one, and it might still be available for years to come. Going through this thread of messages I can see lots of people did they work as well and today there seems to be many of us that have interest in ImGui running on PiX. This is good. I would like to avoid narrowing the scope of my initial intent in order to cater to a today's hit (Pi4) and trash the rest. One way of going forward might be putting up a repo/wiki/??? where all of our ideas and different approaches can be collected and presented. This would allow a random internet wanderer to choose an approach of her liking in getting the ImGui integrated into a project at hand. There are so many dimensions to this, like different hardware, different GLES drivers, different loader, cross compiling, native compiling, .. I feel that we can not support all of it in a way that this upstream ImGui repo remains maintainable. If we can keep bulk of code and guidance out of this repo, but still keep a bare minimum here, we all win, including @ocornut that would not have to deal with hardware support / maintenance of a niche platform. That being said, I probably can not contribute more than I had already in this PR. If you guys feel we can somehow concentrate our knowledge and work for others to benefit from, I will surely try to do my part to the best of my abilities (and time). |
|
Well, i think what i have is already a functional solution. Granted, you have to use cmake, which is not inline with what ImGui does. Anyway i would love to see raspberry support added in the main branch, and i think that it wouldnt be that hard for Omar, since the only imgui code i had to modify was in the main files, thats it. The hard part would be to deal with the modified gl3w, maybe extending support to KMS drivers, or having the code run in an X window. I think a dedicaed wiki section is appropriate. Edit: To clarify: what i mean is that the main library code is untouched. Getting everything to work on raspberry is quite doable. The hard part would be extending support by adding the above features, which may introduce further modifications. With that said those features are possibly outside of imgui's scope. |
|
Once #3951 is merged, both the legacy driver as well as the KMS driver (with or without X) should be supported. Although using KMS without X requires building SDL from source AFAIK, as the packaged version doesn't have the KMSDRM backend apparently. I've added commented out lines along with a bit of explanation to the Makefile, so it's only a matter of uncommenting the appropriate lines: e979ddc#diff-4f9259d043a5fb813bd661ae7f5d4c61e24b0e645c2c8fc0afb7b2823aa091fdR63 With that, having a dedicated wiki page or some other easily found documentation, that explains how to edit the Makefile, might already be enough in the end? What do you think, @hinxx @Pesc0 @audetto ? |
That covers a lot of ground!
For me, comments in the Makefile look enough if all boils down to commenting out two lines, and uncommenting the other two. Essentially you made it so simple that I can not see a need for the script I was signing up for for this PR to come through. The cross compilation of ImGui also involves SDL crosscompile (since its dev pakage is not in the image by default) and process is outlined on one of the SDL wiki pages (https://github.com/libsdl-org/SDL/blob/main/docs/README-raspberrypi.md). It might be enough for the end user of ImGui to follow that process to cross compile ImGui, too. |
Definitely. I am still puzzled that the Raspberry Foundation does not have a clear page documenting this. These are my attempts at getting some clarity around it: https://www.raspberrypi.org/forums/viewtopic.php?f=68&t=304534 And a couple of dummy apps, doing nothing, just to check maximum FPS. https://github.com/audetto/SDL_Demo It is very important to report maximum FPS, so people can quickly check if they are getting the right speed or not. |
|
@lethal-guitar I just looked at #3951, looks real good. One thing: does the SDL example run in an X window? or only fullscreen? Because one quirk i found out is that when running outside X the SDL_window resolution must match the monitor resolution, otherwise the mouse position is all distorted. I achieved this simply by setting the fullscreen flag when initializing the window, this way the specified resolution doesn't matter. Other than that i think a dedicated wiki section would be plenty, since the changes are minimal. Good work! |
|
When I was working on it I couldn't narrow down the issue to the imgui backend, instead I found that weird resolution fix. That looks really close though! Unfortunately right now I can't really try it out, but I'm quite sure that #3950 solved the problem. |
|
While on the course of providing the script, I promised to help building the ImGui for RPi, I realized that it is not something we need anymore given all the recent work by the contributors on this PR that went into the ImGui already. With that being said, I'm not looking into providing any script for compiling ImGui for RPi. At the same time I feel that this PR can be considered as obsolete and can be closed for that matter. |
|
Closing this PR as the solution was provided elsewhere. Thank you all. |
This PR adds support for compiling SDL + opengl3 example on Raspberry Pi (RPi).
RPi uses EGL instead of full openGL.
Makefilewas adjusted to pull in RPi specific headers and libraries found in/opt/vcthroughpkg-config.Since it seems to be no way of automatically detecting compilation for RPi target, it is essentially ARM CPU, a new define called
RPIwas introduced in theMakefileand passed to the compiler.The use of opengl ES 2.0 is also forced through
-DIMGUI_IMPL_OPENGL_ES2specified in theMakefile.The code was compiled natively and the result was successfully tested on the target.