organicmaps/organicmaps-tmp
Archived
13
Fork 1
Code Issues 2.7k Pull requests 182 Projects Releases 65 Wiki Activity Actions

[drape][gl_functions]: Handle ES2 scenario on Linux #2947

Merged
Ferenc- merged 4 commits from wayland_es2 into master 2023-04-11 02:07:21 +00:00
Conversation 5 Commits 4 Files changed 14 +120 -69
Ferenc- commented 2022-07-11 17:57:47 +00:00 (Migrated from github.com)
Copy link

Fixes flathub/app.organicmaps.desktop#1 for devices that are only capable of ES2.

Signed-off-by: Ferenc Géczi ferenc.gm@gmail.com

Fixes flathub/app.organicmaps.desktop#1 for devices that are only capable of ES2. Signed-off-by: Ferenc Géczi <ferenc.gm@gmail.com>
Ferenc- commented 2022-07-11 17:58:52 +00:00 (Migrated from github.com)
Copy link

Let's discuss and test this a bit before potentially merging.

Let's discuss and test this a bit before potentially merging.
biodranik (Migrated from github.com) requested changes 2022-07-11 21:42:09 +00:00
drape/CMakeLists.txt
biodranik (Migrated from github.com) commented 2022-07-11 21:38:49 +00:00

It means that after merging this PR the build will fail on any Linux without EGL installed/available. That's not good for contributors.

It means that after merging this PR the build will fail on any Linux without EGL installed/available. That's not good for contributors.
drape/gl_includes.hpp
biodranik (Migrated from github.com) commented 2022-07-11 21:42:01 +00:00

C++17 supports this:

#if __has_include(<EGL/egl.h>)
...
#endif

We can use it for autodetection of the installed GL version for all platforms.

C++17 supports this: ``` #if __has_include(<EGL/egl.h>) ... #endif ``` We can use it for autodetection of the installed GL version for all platforms.
Ferenc- (Migrated from github.com) reviewed 2022-07-12 06:36:02 +00:00
drape/CMakeLists.txt
Ferenc- (Migrated from github.com) commented 2022-07-12 06:36:02 +00:00

And without this there is no linking to EGL: gl_functions.cpp:260: error: undefined reference to 'eglGetProcAddress'

And without this there is no linking to EGL: `gl_functions.cpp:260: error: undefined reference to 'eglGetProcAddress'`
Ferenc- (Migrated from github.com) reviewed 2022-07-12 06:43:27 +00:00
drape/gl_includes.hpp
Ferenc- (Migrated from github.com) commented 2022-07-12 06:43:26 +00:00

And how would you go about ensuring that the code in this block doesn't suddenly turn into completely dead code, in case the CI environment stops providing that header?

And how would you go about ensuring that the code in this block doesn't suddenly turn into completely dead code, in case the CI environment stops providing that header?
Ferenc- (Migrated from github.com) reviewed 2022-07-12 08:16:13 +00:00
drape/CMakeLists.txt
Ferenc- (Migrated from github.com) commented 2022-07-12 08:16:13 +00:00

Oh you mean that this should be conditional? I will look into that.

Oh you mean that this should be conditional? I will look into that.
biodranik (Migrated from github.com) reviewed 2022-07-13 20:42:35 +00:00
drape/gl_includes.hpp
biodranik (Migrated from github.com) commented 2022-07-13 20:42:35 +00:00

#error ?

`#error` ?
Ferenc- (Migrated from github.com) reviewed 2022-07-14 06:33:39 +00:00
drape/gl_includes.hpp
Ferenc- (Migrated from github.com) commented 2022-07-14 06:33:38 +00:00

But here you said that the code should build for contributors even without EGL, so it can't be an #error.

But [here](https://git.omaps.dev/organicmaps/organicmaps/pulls/2947#discussion_r918371048) you said that the code should build for contributors even without EGL, so it can't be an `#error`.
biodranik (Migrated from github.com) reviewed 2022-07-14 06:43:43 +00:00
drape/gl_includes.hpp
biodranik (Migrated from github.com) commented 2022-07-14 06:43:43 +00:00

The idea is to build with

  • EGL if the header/libs are there
  • GL if the header/libs are there

Or fail with a clear message if there are no GL/EGL headers/libs installed

The idea is to build with - EGL if the header/libs are there - GL if the header/libs are there Or fail with a clear message if there are no GL/EGL headers/libs installed
Ferenc- (Migrated from github.com) reviewed 2022-09-05 15:59:22 +00:00
drape/CMakeLists.txt
Ferenc- (Migrated from github.com) commented 2022-09-05 15:59:22 +00:00

This part is conditional already.

This part is conditional already.
Ferenc- (Migrated from github.com) reviewed 2022-09-05 15:59:43 +00:00
drape/gl_includes.hpp
Ferenc- (Migrated from github.com) commented 2022-09-05 15:59:43 +00:00

I have made EGL conditional.

I have made EGL conditional.
Ferenc- (Migrated from github.com) reviewed 2022-09-05 16:12:56 +00:00
drape/gl_functions.cpp
Ferenc- (Migrated from github.com) commented 2022-09-05 16:12:56 +00:00

Does somebody have an idea why this is?

Does somebody have an idea why this is?
vng (Migrated from github.com) reviewed 2022-09-07 11:07:05 +00:00
drape/gl_functions.cpp
vng (Migrated from github.com) commented 2022-09-07 11:07:05 +00:00

No, but looks like you should check dp::UnpackFormat function, where layout and pixelType vars are initialized. There are magic comments like:

// On OpenGL ES3 GLAlpha is not supported, we use GLRed instead.

So you can experiment with some other constants combination, like GLRed (GL_RED).

No, but looks like you should check dp::UnpackFormat function, where layout and pixelType vars are initialized. There are _magic_ comments like: ``` // On OpenGL ES3 GLAlpha is not supported, we use GLRed instead. ``` So you can experiment with some other constants combination, like GLRed (GL_RED).
vng (Migrated from github.com) reviewed 2022-09-07 11:10:44 +00:00
drape/gl_extensions_list.cpp
vng (Migrated from github.com) commented 2022-09-07 11:10:44 +00:00

"Linux Wayland" is a mobile OS? E.g. if we are checking for OGL ES here, should we enable OMIM_OS_MOBILE in this case? If yes, should we move this checks (ES) above under the OMIM_OS_MOBILE?

"Linux Wayland" is a _mobile_ OS? E.g. if we are checking for OGL ES here, should we enable OMIM_OS_MOBILE in this case? If yes, should we move this checks (ES) above under the OMIM_OS_MOBILE?
vng (Migrated from github.com) reviewed 2022-09-07 11:13:45 +00:00
drape/gl_extensions_list.cpp
vng (Migrated from github.com) commented 2022-09-07 11:13:45 +00:00

But first of all should make this combination (Linux + ES2) works and then prettify defines.

But first of all should make this combination (Linux + ES2) works and then prettify defines.
Ferenc- (Migrated from github.com) reviewed 2022-09-29 16:08:20 +00:00
drape/gl_functions.cpp
Ferenc- (Migrated from github.com) commented 2022-09-29 16:08:19 +00:00

Thanks for the tip @vng.
At first glance, using GLRed does seem to help the GL_INVALID_OPERATION here.
And it turns out that the blurryness is possibly independent from that GL_INVALID_OPERATION here,
it is caused by something else, somewhere else.

Thanks for the tip @vng. At first glance, using `GLRed` does seem to help the `GL_INVALID_OPERATION` here. And it turns out that the blurryness is possibly independent from that `GL_INVALID_OPERATION` here, it is caused by something else, somewhere else.
biodranik (Migrated from github.com) reviewed 2022-09-29 19:00:58 +00:00
drape/gl_extensions_list.cpp
biodranik (Migrated from github.com) commented 2022-09-29 19:00:58 +00:00

+1 to vng

We should choose a proper way for Linux versions. The easiest is probably this one:
Build a separate desktop version with OpenGL, and a mobile version with OpenGL ES.

The more complex way is to build one version that can detect the OpenGL version in runtime and switch appropriately.

+1 to vng We should choose a proper way for Linux versions. The easiest is probably this one: Build a separate desktop version with OpenGL, and a mobile version with OpenGL ES. The more complex way is to build one version that can detect the OpenGL version in runtime and switch appropriately.
programmin1 commented 2022-10-02 06:14:57 +00:00 (Migrated from github.com)
Copy link

I build this for Linux Ubuntu laptop - https://github.com/organicmaps/organicmaps/blob/master/docs/INSTALL.md

However when I transfer to Librem device I get "cannot execute binary file: Exec format error"

probably because Librem phone is an ARM CPU? How do you crosscompile it for ARM?

I build this for Linux Ubuntu laptop - https://github.com/organicmaps/organicmaps/blob/master/docs/INSTALL.md However when I transfer to Librem device I get "cannot execute binary file: Exec format error" probably because Librem phone is an ARM CPU? How do you crosscompile it for ARM?
biodranik commented 2022-10-02 06:36:35 +00:00 (Migrated from github.com)
Copy link

You need to set up the CMake toolchain for the cross-compilation and test if it works. There should be plenty of different manuals for that. Then (in a polished form, to avoid breaking other platforms) these changes can be merged into the main OM repo.

You need to set up the CMake toolchain for the cross-compilation and test if it works. There should be plenty of different manuals for that. Then (in a polished form, to avoid breaking other platforms) these changes can be merged into the main OM repo.
programmin1 commented 2022-10-02 06:40:39 +00:00 (Migrated from github.com)
Copy link

I don't know about configuring "CMake", can you point to where to start with that?

Why is there not an option for multi arch in "tools/unix/build_omim.sh -h" ?

Perhaps putting the 10GB build directory on device and running the same build would build it on the arm computer that is the Librem5 and maybe that would work?

I don't know about configuring "CMake", can you point to where to start with that? Why is there not an option for multi arch in "tools/unix/build_omim.sh -h" ? Perhaps putting the 10GB build directory on device and running the same build would build it on the arm computer that is the Librem5 and maybe that would work?
biodranik commented 2022-10-02 06:43:01 +00:00 (Migrated from github.com)
Copy link

Building it locally would work, you may only need to disable unity builds (or reduce unity batch size) if you have any unexpected crashes. And build only a desktop target, of course.

Building it locally would work, you may only need to disable unity builds (or reduce unity batch size) if you have any unexpected crashes. And build only a `desktop` target, of course.
biodranik (Migrated from github.com) reviewed 2023-02-23 13:42:09 +00:00
drape/CMakeLists.txt
biodranik (Migrated from github.com) commented 2023-02-23 13:39:09 +00:00

It should be conditional on the presence of EGL files, detected by CMake. It requires the installation of mesa packages, right? That also requires updating build documentation. And in reality, it is required only for the desktop target build, e.g. not for the generator_tool.

It should be conditional on the presence of EGL files, detected by CMake. It requires the installation of mesa packages, right? That also requires updating build documentation. And in reality, it is required only for the desktop target build, e.g. not for the generator_tool.
drape/gl_extensions_list.cpp
biodranik (Migrated from github.com) commented 2023-02-23 13:40:12 +00:00

OMIM_OS_MOBILE now uses GLES instead of GL in the code. Should it be enabled also in this case?

OMIM_OS_MOBILE now uses GLES instead of GL in the code. Should it be enabled also in this case?
drape/gl_includes.hpp
biodranik (Migrated from github.com) commented 2023-02-23 13:40:57 +00:00

But not in the CMake, right?

But not in the CMake, right?
biodranik (Migrated from github.com) reviewed 2023-02-23 16:04:07 +00:00
qt/qt_common/map_widget.cpp
@ -245,6 +245,13 @@ void MapWidget::Build()
biodranik (Migrated from github.com) commented 2023-02-23 16:04:07 +00:00

Why did you choose highp here? Should it be chosen according to some dynamic conditions?

Why did you choose highp here? Should it be chosen according to some dynamic conditions?
Ferenc- (Migrated from github.com) reviewed 2023-02-24 07:40:28 +00:00
qt/qt_common/map_widget.cpp
@ -245,6 +245,13 @@ void MapWidget::Build()
Ferenc- (Migrated from github.com) commented 2023-02-24 07:40:27 +00:00

Why did you choose highp here? Should it be chosen according to some dynamic conditions?

Qt dynamically changes that by prepending code to the shader program. so if highp happens to be unsupported, then it gets turned into mediump automatically:

#ifndef GL_FRAGMENT_PRECISION_HIGH                                                                                                             
#define highp mediump                                                                                                                          
#endif
> Why did you choose highp here? Should it be chosen according to some dynamic conditions? Qt dynamically changes that by prepending code to the shader program. so if `highp` happens to be unsupported, then it gets turned into `mediump` automatically: ```` #ifndef GL_FRAGMENT_PRECISION_HIGH #define highp mediump #endif ````
Ferenc- (Migrated from github.com) reviewed 2023-02-24 08:22:08 +00:00
qt/qt_common/map_widget.cpp
@ -245,6 +245,13 @@ void MapWidget::Build()
Ferenc- (Migrated from github.com) commented 2023-02-24 08:22:08 +00:00

Oh and since there is no #version specification in this shader, then when this is running on a hardware where there is only desktop OpenGL available and the default is #version 110, where manual precision specification would normally not be allowed at all, then that case is also covered, because there Qt prepends defines that replace the precision keywords with empty tokens:

#define lowp
#define mediump
#define highp
Oh and since there is no `#version` specification in this shader, then when this is running on a hardware where there is only desktop OpenGL available and the default is `#version 110`, where manual precision specification would normally not be allowed at all, then that case is also covered, because there Qt prepends `define`s that replace the precision keywords with empty tokens: ```` #define lowp #define mediump #define highp ````
biodranik (Migrated from github.com) reviewed 2023-02-24 13:26:15 +00:00
qt/qt_common/map_widget.cpp
@ -245,6 +245,13 @@ void MapWidget::Build()
biodranik (Migrated from github.com) commented 2023-02-24 13:26:14 +00:00

This is clear. What is unclear is why this highp is needed at all here?

This is clear. What is unclear is _why_ this highp is needed at all here?
Ferenc- (Migrated from github.com) reviewed 2023-02-24 16:38:24 +00:00
qt/qt_common/map_widget.cpp
@ -245,6 +245,13 @@ void MapWidget::Build()
Ferenc- (Migrated from github.com) commented 2023-02-24 16:38:24 +00:00

Because otherwise on actual ES 2.0 HW, where GLSL #version 100 is selected instead of #version 110 you don't get a valid shader program. You can check with the official Khronos provided glslangValidator validator, that will report 'float' : type requires declaration of default precision qualifier, which is also what you get in runtime if you try to run this on ES 2.0 HW without the precision qualifier. Qt won't do any magic to make the shader program valid.

Because otherwise on actual ES 2.0 HW, where GLSL `#version 100` is selected instead of `#version 110` you don't get a valid shader program. You can check with the official Khronos provided `glslangValidator` validator, that will report `'float' : type requires declaration of default precision qualifier`, which is also what you get in runtime if you try to run this on ES 2.0 HW without the precision qualifier. Qt won't do any magic to make the shader program valid.
biodranik (Migrated from github.com) reviewed 2023-02-24 22:29:27 +00:00
qt/qt_common/map_widget.cpp
@ -245,6 +245,13 @@ void MapWidget::Build()
biodranik (Migrated from github.com) commented 2023-02-24 22:29:27 +00:00

Thanks for the explanation.

  1. Why GLSL 100 can be selected instead of 110? Or is it the case when 110 is not present/not supported in the system?
  2. What about other our shaders? Are there any similar missing precision issues?
Thanks for the explanation. 1. Why GLSL 100 can be selected instead of 110? Or is it the case when 110 is not present/not supported in the system? 2. What about other our shaders? Are there any similar missing precision issues?
Ferenc- (Migrated from github.com) reviewed 2023-02-25 16:00:39 +00:00
qt/qt_common/map_widget.cpp
@ -245,6 +245,13 @@ void MapWidget::Build()
Ferenc- (Migrated from github.com) commented 2023-02-25 16:00:38 +00:00

Why GLSL 100 can be selected instead of 110? Or is it the case when 110 is not present/not supported in the system?

It is in the GLSL specification for OpenGL ES 2.0. Quote: compilation units that do not include a #version directive will be treated as targeting version 100.
And hardware, that is only capable of ES most of the time does not support #version 110. Because after #version 100, the next step is #version 300 es, there is a table about that, which corresponds to observing HW.

What about other our shaders? Are there any similar missing precision issues?

So vertex shaders don't need it because there is a built-in default precisions there even in ES 2.0. So only ES 2.0 fragment shaders need it. The other fragment shader in map_widget.cpp has so far been explicitly written for #version 150 core where there is a built-in default precision. And the rest of the fragment shaders are generated by gl_shaders_preprocessor.py which inserts a statement which defines the default precision.
So right now, for our use cases I am not aware of any similar issues.

> Why GLSL 100 can be selected instead of 110? Or is it the case when 110 is not present/not supported in the system? It is [in the GLSL specification for OpenGL ES 2.0](https://registry.khronos.org/OpenGL/specs/es/2.0/GLSL_ES_Specification_1.00.pdf). Quote: `compilation units that do not include a #version directive will be treated as targeting version 100.` And hardware, that is only capable of ES most of the time does not support `#version 110`. Because after `#version 100`, the next step is `#version 300 es`, there is a [table about that](https://en.wikipedia.org/wiki/OpenGL_Shading_Language#Versions), which corresponds to observing HW. > What about other our shaders? Are there any similar missing precision issues? So vertex shaders don't need it because there is a built-in default precisions there even in ES 2.0. So only ES 2.0 fragment shaders need it. The other fragment shader in `map_widget.cpp` has so far been explicitly written for `#version 150 core` where there is a built-in default precision. And the rest of the fragment shaders are generated by `gl_shaders_preprocessor.py` which [inserts a statement which defines the default precision](https://github.com/organicmaps/organicmaps/blob/master/shaders/gl_shaders_preprocessor.py#L137). So right now, for our use cases I am not aware of any similar issues.
biodranik (Migrated from github.com) reviewed 2023-02-25 21:21:20 +00:00
.github/workflows/linux-check.yaml
biodranik (Migrated from github.com) commented 2023-02-25 21:13:10 +00:00

Should docs/INSTALL.md be updated? Is it feasible/does it worth it to make mesa a conditional dependency? What are pros and cons?

Should docs/INSTALL.md be updated? Is it feasible/does it worth it to make mesa a conditional dependency? What are pros and cons?
drape/gl_extensions_list.cpp
biodranik (Migrated from github.com) commented 2023-02-25 21:14:59 +00:00

Or, alternatively, should we rewrite/remove OMIM_OS_MOBILE to dynamically initialize GLES if it's present, and fall back to GL if it's not?

Or, alternatively, should we rewrite/remove OMIM_OS_MOBILE to dynamically initialize GLES if it's present, and fall back to GL if it's not?
biodranik (Migrated from github.com) commented 2023-02-25 21:16:54 +00:00

Can you please add similar comments to other #endif clauses here for easier readability? It looks complex now.

Can you please add similar comments to other `#endif` clauses here for easier readability? It looks complex now.
drape/gl_functions.cpp
biodranik (Migrated from github.com) commented 2023-02-25 21:18:16 +00:00 
    glFlushMappedBufferRangeFn = (TglFlushMappedBufferRangeFn)eglGetProcAddress("glFlushMappedBufferRangeEXT");
```suggestion glFlushMappedBufferRangeFn = (TglFlushMappedBufferRangeFn)eglGetProcAddress("glFlushMappedBufferRangeEXT"); ```
qt/main.cpp
@ -169,19 +170,8 @@ int main(int argc, char * argv[])
int returnCode = -1;
if (eulaAccepted) // User has accepted EULA
{
biodranik (Migrated from github.com) commented 2023-02-25 21:21:02 +00:00

Shouldn't the available es2 or es3 version be detected in runtime automatically?

Shouldn't the available es2 or es3 version be detected in runtime automatically?
qt/qt_common/map_widget.cpp
@ -245,6 +245,13 @@ void MapWidget::Build()
biodranik (Migrated from github.com) commented 2023-02-25 21:21:14 +00:00

Thank you for the details!

Thank you for the details!
Ferenc- (Migrated from github.com) reviewed 2023-02-28 20:55:10 +00:00
drape/gl_functions.cpp
Ferenc- (Migrated from github.com) commented 2023-02-28 20:55:10 +00:00

The rest of this file uses that indentation? Shouldn't we follow that?

The rest of this file uses that indentation? Shouldn't we follow that?
Ferenc- (Migrated from github.com) reviewed 2023-02-28 20:56:41 +00:00
qt/main.cpp
@ -169,19 +170,8 @@ int main(int argc, char * argv[])
int returnCode = -1;
if (eulaAccepted) // User has accepted EULA
{
Ferenc- (Migrated from github.com) commented 2023-02-28 20:56:41 +00:00

Ideally yes. And I think that should be a topic of a separate PR, because this PR is already very old.
Do you think it is acceptable to do that in a separate PR?

Ideally yes. And I think that should be a topic of a separate PR, because this PR is already very old. Do you think it is acceptable to do that in a separate PR?
Ferenc- (Migrated from github.com) reviewed 2023-02-28 20:59:50 +00:00
drape/gl_extensions_list.cpp
Ferenc- (Migrated from github.com) commented 2023-02-28 20:59:50 +00:00

I think I would remove the dependency on EGL, and then this would not be so complex anymore.

I think I would remove the dependency on EGL, and then this would not be so complex anymore.
Ferenc- (Migrated from github.com) reviewed 2023-02-28 21:00:09 +00:00
.github/workflows/linux-check.yaml
Ferenc- (Migrated from github.com) commented 2023-02-28 21:00:09 +00:00

I think I would remove the dependency on EGL, and then no change is needed here.

I think I would remove the dependency on EGL, and then no change is needed here.
Ferenc- (Migrated from github.com) reviewed 2023-02-28 21:12:50 +00:00
drape/gl_extensions_list.cpp
Ferenc- (Migrated from github.com) commented 2023-02-28 21:12:50 +00:00

Or, alternatively, should we rewrite/remove OMIM_OS_MOBILE to dynamically initialize GLES if it's present, and fall back to GL if it's not?

I am not entirely sure what you mean by dynamically initialize GLES in this context, but OMIM_OS_MOBILE should probably be removed from here. And maybe even completely. In the past it made sense to use OMIM_OS_MOBILE for switching between the mobile and the desktop wikipedia base url, but by now it is the same everywhere.
So I would consider removing OMIM_OS_MOBILE completely in a separate PR. What do you think?

> Or, alternatively, should we rewrite/remove OMIM_OS_MOBILE to dynamically initialize GLES if it's present, and fall back to GL if it's not? I am not entirely sure what you mean by dynamically initialize GLES in this context, but `OMIM_OS_MOBILE` should probably be removed from here. And maybe even completely. In the past it made sense to use `OMIM_OS_MOBILE` for switching between the mobile and the desktop wikipedia base url, but by now it is the same everywhere. So I would consider removing `OMIM_OS_MOBILE` completely in a separate PR. What do you think?
biodranik (Migrated from github.com) reviewed 2023-02-28 22:31:15 +00:00
drape/gl_extensions_list.cpp
biodranik (Migrated from github.com) commented 2023-02-28 22:31:15 +00:00

Now on Android and iOS GLES is initialized via OMIM_OS_MOBILE

Should we do it differently now? GLES may not be available on desktop platforms. Btw, we can remove it completely on iOS too, it should use Metal instead. But for Android it can be used as a fallback from Vulkan for <24 APIs and also to work-around some Vulkan issues.

Now on Android and iOS GLES is initialized via OMIM_OS_MOBILE Should we do it differently now? GLES may not be available on desktop platforms. Btw, we can remove it completely on iOS too, it should use Metal instead. But for Android it can be used as a fallback from Vulkan for <24 APIs and also to work-around some Vulkan issues.
biodranik (Migrated from github.com) reviewed 2023-02-28 22:32:09 +00:00
drape/gl_extensions_list.cpp
biodranik (Migrated from github.com) commented 2023-02-28 22:32:09 +00:00

Regarding Wikipedia, why do you think it's better to remove different links? One less redirect means a better UX.

Regarding Wikipedia, why do you think it's better to remove different links? One less redirect means a better UX.
Ferenc- (Migrated from github.com) reviewed 2023-03-01 17:25:20 +00:00
drape/gl_extensions_list.cpp
Ferenc- (Migrated from github.com) commented 2023-03-01 17:25:20 +00:00

Now on Android and iOS GLES is initialized via OMIM_OS_MOBILE

My observation is that for android it is iniitialized via it's own OMIM_OS_ANDROID so for GLES purposes OMIM_OS_MOBILE should probalby be replaced with something like OMIM_OS_IPHONE.

One less redirect means a better UX

In my experience, If one opens the mobile version on a desktop, then there is no redirect, and the latest desktop experience is pretty close to the mobile one.
In any case, it might make sense to keep OMIM_OS_MOBILE for the wiki links, while the GLES init should probably be refactored.

> Now on Android and iOS GLES is initialized via OMIM_OS_MOBILE My observation is that for android it is iniitialized via it's own `OMIM_OS_ANDROID` so for GLES purposes `OMIM_OS_MOBILE` should probalby be replaced with something like `OMIM_OS_IPHONE`. > One less redirect means a better UX In my experience, If one opens the mobile version on a desktop, then there is no redirect, and the [latest desktop experience](https://wikimediafoundation.org/news/2023/01/18/wikipedia-gets-a-fresh-new-look-first-desktop-update-in-a-decade-puts-usability-at-the-forefront/) is pretty close to the mobile one. In any case, it might make sense to keep `OMIM_OS_MOBILE` for the wiki links, while the GLES init should probably be refactored.
Ferenc- (Migrated from github.com) reviewed 2023-03-04 08:12:29 +00:00
qt/main.cpp
@ -169,19 +170,8 @@ int main(int argc, char * argv[])
int returnCode = -1;
if (eulaAccepted) // User has accepted EULA
{
Ferenc- (Migrated from github.com) commented 2023-03-04 08:12:28 +00:00

It will probably prolong the review process, but I have addedd code for that anyway.

It will probably prolong the review process, but I have addedd code for that anyway.
Ferenc- (Migrated from github.com) reviewed 2023-03-04 08:13:02 +00:00
drape/gl_functions.cpp
Ferenc- (Migrated from github.com) commented 2023-03-04 08:13:01 +00:00

This code has changed regardless, please check again.

This code has changed regardless, please check again.
Ferenc- (Migrated from github.com) reviewed 2023-03-04 08:13:40 +00:00
drape/gl_extensions_list.cpp
Ferenc- (Migrated from github.com) commented 2023-03-04 08:13:40 +00:00

Anyhow, now that macro directive has been removed.

Anyhow, now that macro directive has been removed.
Ferenc- (Migrated from github.com) reviewed 2023-03-04 08:14:16 +00:00
.github/workflows/linux-check.yaml
Ferenc- (Migrated from github.com) commented 2023-03-04 08:14:16 +00:00

For now all removed.

For now all removed.
biodranik (Migrated from github.com) reviewed 2023-03-11 11:40:08 +00:00
biodranik (Migrated from github.com) left a comment

The idea is good. But I afraid of the slower startup caused by double GL initialization. Can it be optimized?

The idea is good. But I afraid of the slower startup caused by double GL initialization. Can it be optimized?
drape/gl_functions.cpp
biodranik (Migrated from github.com) commented 2023-03-11 11:39:29 +00:00

Is it portable?

Is it portable?
qt/draw_widget.cpp
biodranik (Migrated from github.com) commented 2023-03-11 11:38:34 +00:00

Why?

Why?
qt/main.cpp
@ -169,19 +170,8 @@ int main(int argc, char * argv[])
int returnCode = -1;
if (eulaAccepted) // User has accepted EULA
{
biodranik (Migrated from github.com) commented 2023-03-11 11:35:31 +00:00 
        LOG(LINFO, ("Context is LibGLES"));
```suggestion LOG(LINFO, ("Context is LibGLES")); ```
biodranik (Migrated from github.com) commented 2023-03-11 11:36:05 +00:00

It's not our braces style )

It's not our braces style )
biodranik (Migrated from github.com) commented 2023-03-11 11:36:40 +00:00 
        // TODO: Fix the ES3 code path with ES3 compatible shader code.
        apiOpenGLES3 = false;
```suggestion // TODO: Fix the ES3 code path with ES3 compatible shader code. apiOpenGLES3 = false; ```
biodranik (Migrated from github.com) commented 2023-03-11 11:37:41 +00:00

Ditto

Ditto
biodranik (Migrated from github.com) commented 2023-03-11 11:38:03 +00:00

These comments are redundant )

These comments are redundant )
qt/qt_common/map_widget.cpp
biodranik (Migrated from github.com) commented 2023-03-11 11:33:44 +00:00

If it's used only here, then define can be avoided and this line can be used directly in if else.

If it's used only here, then define can be avoided and this line can be used directly in if else.
Ferenc- (Migrated from github.com) reviewed 2023-03-12 18:49:39 +00:00
qt/main.cpp
@ -169,19 +170,8 @@ int main(int argc, char * argv[])
int returnCode = -1;
if (eulaAccepted) // User has accepted EULA
{
Ferenc- (Migrated from github.com) commented 2023-03-12 18:49:39 +00:00

Removed.

Removed.
Ferenc- (Migrated from github.com) reviewed 2023-03-12 18:52:22 +00:00
qt/qt_common/map_widget.cpp
Ferenc- (Migrated from github.com) commented 2023-03-12 18:52:22 +00:00

Changed.

Changed.
Ferenc- (Migrated from github.com) reviewed 2023-03-12 18:53:23 +00:00
qt/main.cpp
@ -169,19 +170,8 @@ int main(int argc, char * argv[])
int returnCode = -1;
if (eulaAccepted) // User has accepted EULA
{
Ferenc- (Migrated from github.com) commented 2023-03-12 18:53:23 +00:00

Changed.

Changed.
Ferenc- (Migrated from github.com) reviewed 2023-03-12 18:56:14 +00:00
qt/main.cpp
@ -169,19 +170,8 @@ int main(int argc, char * argv[])
int returnCode = -1;
if (eulaAccepted) // User has accepted EULA
{
Ferenc- (Migrated from github.com) commented 2023-03-12 18:56:14 +00:00

Changed.

Changed.
Ferenc- (Migrated from github.com) reviewed 2023-03-12 18:57:15 +00:00
qt/main.cpp
@ -169,19 +170,8 @@ int main(int argc, char * argv[])
int returnCode = -1;
if (eulaAccepted) // User has accepted EULA
{
Ferenc- (Migrated from github.com) commented 2023-03-12 18:57:15 +00:00

Changed.

Changed.
Ferenc- (Migrated from github.com) reviewed 2023-03-12 18:57:44 +00:00
qt/main.cpp
@ -169,19 +170,8 @@ int main(int argc, char * argv[])
int returnCode = -1;
if (eulaAccepted) // User has accepted EULA
{
Ferenc- (Migrated from github.com) commented 2023-03-12 18:57:44 +00:00

Changed.

Changed.
Ferenc- (Migrated from github.com) reviewed 2023-03-26 11:45:54 +00:00
drape/gl_functions.cpp
Ferenc- (Migrated from github.com) commented 2023-03-26 11:45:54 +00:00

At the moment I can't think of any typical scenario in which it wouldn't work.
Perhaps some rare corner cases, in which there are multiple displays attached to different GPUs,
which can only do ES 2.0.
But ES 2.0 only hardware is rarer and rarer anyway, any such corner cases are unsupported anyway.
At the moment I don't consider it a high priority to cover corner cases in which this wouldn't work.
I think if the project wants to keep up with technological development at large,
then we should make this ES 2.0 support leap as soon as possible, and move on to ES 3.0 and beyond,
and see if anyone is even interested in some obscure, old setup.

At the moment I can't think of any typical scenario in which it wouldn't work. Perhaps some rare corner cases, in which there are multiple displays attached to different GPUs, which can only do ES 2.0. But ES 2.0 only hardware is rarer and rarer anyway, any such corner cases are unsupported anyway. At the moment I don't consider it a high priority to cover corner cases in which this wouldn't work. I think if the project wants to keep up with technological development at large, then we should make this ES 2.0 support leap as soon as possible, and move on to ES 3.0 and beyond, and see if anyone is even interested in some obscure, old setup.
Ferenc- (Migrated from github.com) reviewed 2023-03-26 11:50:28 +00:00
qt/draw_widget.cpp
Ferenc- (Migrated from github.com) commented 2023-03-26 11:50:28 +00:00

Nothing relies on 2, 1 in a desktop GL scenario, and AFAIK the proper way of requesting ES 2.0 is this.
And all this setVersion call is just a polite request anyway, which might or might not be abided.
So it's not super relevant anyway.
If it is a big eye soar, then I can re-test with 2, 0, I just don't see much value in it.

Nothing relies on `2, 1` in a desktop GL scenario, and AFAIK the proper way of requesting ES 2.0 is this. And all this `setVersion` call is just a polite request anyway, which might or might not be abided. So it's not super relevant anyway. If it is a big eye soar, then I can re-test with `2, 0`, I just don't see much value in it.
biodranik (Migrated from github.com) reviewed 2023-03-26 15:27:02 +00:00
qt/draw_widget.cpp
biodranik (Migrated from github.com) commented 2023-03-26 15:27:02 +00:00

Why it was chosen in the first place instead of 2.0? Was there some reasoning in commit/pr for mapsme/omim repo?

Why it was chosen in the first place instead of 2.0? Was there some reasoning in commit/pr for mapsme/omim repo?
biodranik (Migrated from github.com) reviewed 2023-03-26 15:28:52 +00:00
drape/gl_functions.cpp
biodranik (Migrated from github.com) commented 2023-03-26 15:28:52 +00:00
  1. Does it make sense to support GLES and (non-ES) GL at the same time, for desktops and mobile linux?
  2. Where only ES2 is used? How many devices won't be able to run OM if we drop it?
1. Does it make sense to support GLES and (non-ES) GL at the same time, for desktops and mobile linux? 2. Where only ES2 is used? How many devices won't be able to run OM if we drop it?
Ferenc- (Migrated from github.com) reviewed 2023-03-26 18:16:48 +00:00
qt/draw_widget.cpp
Ferenc- (Migrated from github.com) commented 2023-03-26 18:16:48 +00:00

From the git history It is unclear to me. It was simply selected in 3434dd895e,
for unknown reasons, even though the commit was not even about ES2, but desktop OpenGL 3.
And previous to that commit there was no call to QSurfaceFormat::setVersion in the codebase whatsoever.

From the git history It is unclear to me. It was simply selected in https://git.omaps.dev/organicmaps/organicmaps/commit/3434dd895e5cac2ef9167b3118a5b006072ab9c1, for unknown reasons, even though the commit was not even about ES2, but desktop OpenGL 3. And previous to that commit there was no call to `QSurfaceFormat::setVersion` in the codebase whatsoever.
Ferenc- (Migrated from github.com) reviewed 2023-03-26 18:29:57 +00:00
qt/draw_widget.cpp
Ferenc- (Migrated from github.com) commented 2023-03-26 18:29:56 +00:00

And if I may add my commentary, to 3434dd895e, then there it already appears to be inconsistent with the shader codes IMHO. Becase both the fragment and the vertex shaders were added without version directive, which AFAIK had to default to #version 110, and that does not correspond to 2, 1 but to 2. 0, if I am not mistaken.

And if I may add my commentary, to https://git.omaps.dev/organicmaps/organicmaps/commit/3434dd895e5cac2ef9167b3118a5b006072ab9c1, then there it already appears to be inconsistent with the shader codes IMHO. Becase both the fragment and the vertex shaders were added without version directive, which AFAIK had to default to `#version 110`, and that does not correspond to `2, 1` but to `2. 0`, if I am not mistaken.
vng (Migrated from github.com) reviewed 2023-03-26 18:54:30 +00:00
qt/main.cpp
@ -247,7 +262,7 @@ int main(int argc, char * argv[])
#endif // BUILD_DESIGNER
Framework framework(frameworkParams);
vng (Migrated from github.com) commented 2023-03-26 18:53:41 +00:00

Do we need it here since you moved initialization code into MapWidget::initializeGL() ?
Or there is no problem to call setDefaultFormat twice?

Do we need it here since you moved initialization code into MapWidget::initializeGL() ? Or there is no problem to call setDefaultFormat twice?
qt/qt_common/map_widget.cpp
vng (Migrated from github.com) commented 2023-03-26 18:54:19 +00:00

Looks like this var can be local here, not in MapWidget member scope.

Looks like this var can be local here, not in MapWidget member scope.
biodranik (Migrated from github.com) reviewed 2023-03-27 00:10:59 +00:00
qt/draw_widget.cpp
biodranik (Migrated from github.com) commented 2023-03-27 00:10:59 +00:00

The old code was working with the desktop OpenGL 2.1 and then (after the mentioned patch) with 3.0. ES is not related here.

Requesting (non-ES) OpenGL 2.1 instead of 2.0 is a good idea.

The old code was working with the desktop OpenGL 2.1 and then (after the mentioned patch) with 3.0. ES is not related here. Requesting (non-ES) OpenGL 2.1 instead of 2.0 is a good idea.
qt/qt_common/map_widget.cpp
@ -340,0 +390,4 @@
fmt.setProfile(QSurfaceFormat::CompatibilityProfile);
fmt.setVersion(2, 1);
}
QSurfaceFormat::setDefaultFormat(fmt);
biodranik (Migrated from github.com) commented 2023-03-27 00:04:23 +00:00

Is this approach portable also to Windows?

Is this approach portable also to Windows?
biodranik (Migrated from github.com) reviewed 2023-03-27 06:40:30 +00:00
drape/gl_extensions_list.cpp
biodranik (Migrated from github.com) commented 2023-03-27 06:23:51 +00:00

It is OpenGL 2.0, OpenGL 3.0, and OpenGLES3 branch, right?

It is OpenGL 2.0, OpenGL 3.0, and OpenGLES3 branch, right?
drape/gl_functions.cpp
biodranik (Migrated from github.com) commented 2023-03-27 06:22:34 +00:00

LOG(CRITICAL, "") should stop execution here in debug and release.

LOG(CRITICAL, "") should stop execution here in debug and release.
qt/qt_common/map_widget.cpp
biodranik (Migrated from github.com) commented 2023-03-27 06:29:15 +00:00 
        if (context()->hasExtension(QByteArray::fromStdString(requiredExtension)))
          LOG(LDEBUG, ("Found OpenGL ES 2.0 extension: ", requiredExtension));
        else
          LOG(LCRITICAL, ("A required OpenGL ES 2.0 extension is missing: ", requiredExtension));
```suggestion if (context()->hasExtension(QByteArray::fromStdString(requiredExtension))) LOG(LDEBUG, ("Found OpenGL ES 2.0 extension: ", requiredExtension)); else LOG(LCRITICAL, ("A required OpenGL ES 2.0 extension is missing: ", requiredExtension)); ```
biodranik (Migrated from github.com) commented 2023-03-27 06:39:28 +00:00 
    fmt.setVersion(2, 1);
```suggestion fmt.setVersion(2, 1); ```
qt/qt_common/map_widget.cpp
biodranik (Migrated from github.com) commented 2023-03-27 06:30:25 +00:00

Are different shaders used now?

Are different shaders used now?
qt/qt_common/map_widget.cpp
biodranik (Migrated from github.com) commented 2023-03-27 06:39:57 +00:00

Isn't the default format already defined at this moment?

Isn't the default format already defined at this moment?
Ferenc- (Migrated from github.com) reviewed 2023-03-27 17:01:26 +00:00
qt/qt_common/map_widget.cpp
Ferenc- (Migrated from github.com) commented 2023-03-27 17:01:25 +00:00

All right then, reverted to 2, 1, but do we know why?

All right then, reverted to `2, 1`, but do we know why?
Ferenc- (Migrated from github.com) reviewed 2023-03-27 17:02:18 +00:00
qt/qt_common/map_widget.cpp
Ferenc- (Migrated from github.com) commented 2023-03-27 17:02:17 +00:00

No, I didn't consider it part of this PR.

No, I didn't consider it part of this PR.
Ferenc- (Migrated from github.com) reviewed 2023-03-27 17:03:02 +00:00
qt/qt_common/map_widget.cpp
Ferenc- (Migrated from github.com) commented 2023-03-27 17:03:02 +00:00

Yes, we are updating it here, just to be sure, that any consequent surfaces will use it too.

Yes, we are updating it here, just to be sure, that any consequent surfaces will use it too.
Ferenc- (Migrated from github.com) reviewed 2023-03-27 17:04:10 +00:00
qt/qt_common/map_widget.cpp
Ferenc- (Migrated from github.com) commented 2023-03-27 17:04:09 +00:00

The original idea was to stop execution in the else branch in the final version.
I have left that one out for test purposes, but anyhow changed it for now.

The original idea was to stop execution in the `else` branch in the final version. I have left that one out for test purposes, but anyhow changed it for now.
Ferenc- (Migrated from github.com) reviewed 2023-03-27 17:09:16 +00:00
drape/gl_extensions_list.cpp
Ferenc- (Migrated from github.com) commented 2023-03-27 17:09:15 +00:00

No OpenGL 2.0 is also handled with 2.0 ES in the previous one. In any case I have updated the comment.

No OpenGL 2.0 is also handled with 2.0 ES in the previous one. In any case I have updated the comment.
Ferenc- (Migrated from github.com) reviewed 2023-03-27 17:12:15 +00:00
qt/qt_common/map_widget.cpp
@ -340,0 +390,4 @@
fmt.setProfile(QSurfaceFormat::CompatibilityProfile);
fmt.setVersion(2, 1);
}
QSurfaceFormat::setDefaultFormat(fmt);
Ferenc- (Migrated from github.com) commented 2023-03-27 17:12:15 +00:00

As portable as any other Qt functions. This is Qt code like the rest. The doc doesn't state any restrictions.

As portable as any other Qt functions. This is Qt code like the rest. The [doc doesn't state any restrictions.](https://doc.qt.io/qt-5/qopenglcontext.html#isOpenGLES)
Ferenc- (Migrated from github.com) reviewed 2023-03-27 17:19:36 +00:00
qt/qt_common/map_widget.cpp
Ferenc- (Migrated from github.com) commented 2023-03-27 17:19:36 +00:00

m_apiOpenGLES3 is also used in CreateEngine() and any locally set value should somehow get out of the scope of initializeGL() which cannot change signature, because it is an implementation of a virtual parent function.
So if we can't take in output parameter, and can't return anything then, I don't see any better option than a member field.

`m_apiOpenGLES3` is also used in `CreateEngine()` and any locally set value should somehow get out of the scope of `initializeGL()` which cannot change signature, because it is an [implementation of a virtual parent function](https://doc.qt.io/qt-5/qglwidget.html#initializeGL). So if we can't take in output parameter, and can't return anything then, I don't see any better option than a member field.
Ferenc- (Migrated from github.com) reviewed 2023-03-27 17:20:56 +00:00
qt/main.cpp
@ -247,7 +262,7 @@ int main(int argc, char * argv[])
#endif // BUILD_DESIGNER
Framework framework(frameworkParams);
Ferenc- (Migrated from github.com) commented 2023-03-27 17:20:56 +00:00

There is absolutely no limit how many times you can call setDefaultFormat, every time you call it it will be in effect for the next surface that is either created manually or automatically by Qt.

There is absolutely no limit how many times you can call `setDefaultFormat`, every time you call it it will be in effect for the next surface that is either created manually or automatically by Qt.
biodranik (Migrated from github.com) reviewed 2023-03-27 17:22:17 +00:00
qt/qt_common/map_widget.cpp
biodranik (Migrated from github.com) commented 2023-03-27 17:22:17 +00:00

Yes. Because supporting OpenGL 2.1 instead of 2.0 is better. And it was already working for many years.

Yes. Because supporting OpenGL 2.1 instead of 2.0 is better. And it was already working for many years.
vng (Migrated from github.com) approved these changes 2023-03-27 20:58:12 +00:00
vng (Migrated from github.com) left a comment

LGTM. @pastk Would you like to test on your Linux?

LGTM. @pastk Would you like to test on your Linux?
Ferenc- (Migrated from github.com) reviewed 2023-03-28 16:40:31 +00:00
drape/gl_functions.cpp
Ferenc- (Migrated from github.com) commented 2023-03-28 16:40:31 +00:00

Done.

Done.
biodranik (Migrated from github.com) reviewed 2023-04-05 16:21:44 +00:00
biodranik (Migrated from github.com) left a comment

Great, we're almost there. But it is not easy to understand now how es2/es3 are mixed though...

Great, we're almost there. But it is not easy to understand now how es2/es3 are mixed though...
drape/gl_extensions_list.cpp
biodranik (Migrated from github.com) commented 2023-04-05 15:47:26 +00:00 
  SetExtension(VertexArrayObject, true);
  SetExtension(MapBufferRange, true);

  if (apiVersion == dp::ApiVersion::OpenGLES2)
    SetExtension(MapBuffer, true);
```suggestion SetExtension(VertexArrayObject, true); SetExtension(MapBufferRange, true); if (apiVersion == dp::ApiVersion::OpenGLES2) SetExtension(MapBuffer, true); ```
biodranik (Migrated from github.com) commented 2023-04-05 15:57:36 +00:00

Actually, is there a case for Linux where OpenGLES2 is still used? Can we remove it completely?

Actually, is there a case for Linux where OpenGLES2 is still used? Can we remove it completely?
drape/gl_functions.cpp
@ -260,0 +263,4 @@
glDeleteVertexArrayFn = (TglDeleteVertexArrayFn)dlsym(libhandle,"glDeleteVertexArraysOES");
glMapBufferFn = (TglMapBufferFn)dlsym(libhandle, "glMapBufferOES");
glUnmapBufferFn = (TglUnmapBufferFn)dlsym(libhandle, "glUnmapBufferOES");
glMapBufferRangeFn = (TglMapBufferRangeFn)dlsym(libhandle, "glMapBufferRangeEXT");
biodranik (Migrated from github.com) commented 2023-04-05 15:48:19 +00:00

Isn't mapbuffer available only for GLES 2.0 (according to the if above)?

Isn't mapbuffer available only for GLES 2.0 (according to the if above)?
qt/main.cpp
biodranik (Migrated from github.com) commented 2023-04-05 15:49:25 +00:00 
using namespace std;
```suggestion using namespace std; ```
qt/qt_common/map_widget.cpp
biodranik (Migrated from github.com) commented 2023-04-05 15:57:07 +00:00

Is GLES2 used now only on Mac? Can we use GL3+/GLES3 everywhere?

I've tested es3 on my 2014 Macbook Pro with Big Sur, and it works fine.

Is GLES2 used now only on Mac? Can we use GL3+/GLES3 everywhere? I've tested es3 on my 2014 Macbook Pro with Big Sur, and it works fine.
biodranik (Migrated from github.com) commented 2023-04-05 16:14:43 +00:00 
    QOpenGLFunctions * glf = context()->functions();
    LOG(LINFO, ("Vendor:", glf->glGetString(GL_VENDOR),
                "\nRenderer:", glf->glGetString(GL_RENDERER),
                "\nVersion:", glf->glGetString(GL_VERSION),
                "\nShading language version:", glf->glGetString(GL_SHADING_LANGUAGE_VERSION),
                "\nExtensions:", glf->glGetString(GL_EXTENSIONS)));
```suggestion QOpenGLFunctions * glf = context()->functions(); LOG(LINFO, ("Vendor:", glf->glGetString(GL_VENDOR), "\nRenderer:", glf->glGetString(GL_RENDERER), "\nVersion:", glf->glGetString(GL_VERSION), "\nShading language version:", glf->glGetString(GL_SHADING_LANGUAGE_VERSION), "\nExtensions:", glf->glGetString(GL_EXTENSIONS))); ```
biodranik (Migrated from github.com) commented 2023-04-05 16:17:06 +00:00 
      for (auto & requiredExtension : requiredExtensions)
```suggestion for (auto & requiredExtension : requiredExtensions) ```
biodranik (Migrated from github.com) commented 2023-04-05 16:18:12 +00:00 
        if (context()->hasExtension(QByteArray::fromStdString(requiredExtension)))
```suggestion if (context()->hasExtension(QByteArray::fromStdString(requiredExtension))) ```
biodranik (Migrated from github.com) commented 2023-04-05 16:18:42 +00:00 
          LOG(LCRITICAL, ("A required OpenGL ES 2.0 extension is missing:", requiredExtension));
```suggestion LOG(LCRITICAL, ("A required OpenGL ES 2.0 extension is missing:", requiredExtension)); ```
biodranik (Migrated from github.com) commented 2023-04-05 16:20:53 +00:00
```suggestion ```
qt/qt_common/map_widget.cpp
@ -340,3 +394,4 @@
m_contextFactory.reset(new QtOGLContextFactory(context()));
emit BeforeEngineCreation();
biodranik (Migrated from github.com) commented 2023-04-05 16:19:46 +00:00

Does it mean that GLES3 is not working yet on Linux?

Does it mean that GLES3 is not working yet on Linux?
Ferenc- (Migrated from github.com) reviewed 2023-04-07 12:50:17 +00:00
qt/qt_common/map_widget.cpp
Ferenc- (Migrated from github.com) commented 2023-04-07 12:50:17 +00:00

Removed

Removed
Ferenc- (Migrated from github.com) reviewed 2023-04-07 12:51:13 +00:00
qt/qt_common/map_widget.cpp
Ferenc- (Migrated from github.com) commented 2023-04-07 12:51:13 +00:00

The trailing whitespace has been removed.

The trailing whitespace has been removed.
Ferenc- (Migrated from github.com) reviewed 2023-04-07 12:51:55 +00:00
qt/qt_common/map_widget.cpp
Ferenc- (Migrated from github.com) commented 2023-04-07 12:51:54 +00:00

Space added.

Space added.
Ferenc- (Migrated from github.com) reviewed 2023-04-07 12:52:31 +00:00
qt/qt_common/map_widget.cpp
Ferenc- (Migrated from github.com) commented 2023-04-07 12:52:31 +00:00

Spaces supplied.

Spaces supplied.
Ferenc- (Migrated from github.com) reviewed 2023-04-07 12:54:17 +00:00
qt/main.cpp
Ferenc- (Migrated from github.com) commented 2023-04-07 12:54:16 +00:00

Empty line has been removed.

Empty line has been removed.
Ferenc- (Migrated from github.com) reviewed 2023-04-07 12:59:37 +00:00
qt/qt_common/map_widget.cpp
Ferenc- (Migrated from github.com) commented 2023-04-07 12:59:37 +00:00

I have eliminated the variables.

I have eliminated the variables.
Ferenc- (Migrated from github.com) reviewed 2023-04-07 13:15:30 +00:00
qt/qt_common/map_widget.cpp
Ferenc- (Migrated from github.com) commented 2023-04-07 13:15:30 +00:00

OK, then I have flipped GLES3 on for Mac.

OK, then I have flipped `GLES3` on for Mac.
Ferenc- (Migrated from github.com) reviewed 2023-04-07 15:34:44 +00:00
drape/gl_extensions_list.cpp
Ferenc- (Migrated from github.com) commented 2023-04-07 15:34:44 +00:00

Apparently this conditional statement is not even needed. Removed.

Apparently this conditional statement is not even needed. Removed.
Ferenc- (Migrated from github.com) reviewed 2023-04-07 15:38:29 +00:00
drape/gl_functions.cpp
@ -260,0 +263,4 @@
glDeleteVertexArrayFn = (TglDeleteVertexArrayFn)dlsym(libhandle,"glDeleteVertexArraysOES");
glMapBufferFn = (TglMapBufferFn)dlsym(libhandle, "glMapBufferOES");
glUnmapBufferFn = (TglUnmapBufferFn)dlsym(libhandle, "glUnmapBufferOES");
glMapBufferRangeFn = (TglMapBufferRangeFn)dlsym(libhandle, "glMapBufferRangeEXT");
Ferenc- (Migrated from github.com) commented 2023-04-07 15:38:29 +00:00

This is the 2.0 branch, and mapbuffer is not exclusive to 2.0.

This is the 2.0 branch, and mapbuffer is not exclusive to 2.0.
biodranik (Migrated from github.com) approved these changes 2023-04-08 21:04:46 +00:00
biodranik (Migrated from github.com) left a comment

It's ok, but I still don't understand why is there TODO for gles3 for Linux.

It's ok, but I still don't understand why is there TODO for gles3 for Linux.
drape/gl_functions.cpp
biodranik (Migrated from github.com) commented 2023-04-08 20:38:40 +00:00
  1. Why not RTLD_LAZY ?
  2. Is dlclose needed after dlsym calls below?
1. Why not RTLD_LAZY ? 2. Is dlclose needed after dlsym calls below?
drape/gl_functions.cpp
@ -260,0 +260,4 @@
LOG(LCRITICAL, ("Failed to open libGL.so.1:", dlerror()));
glGenVertexArraysFn = (TglGenVertexArraysFn)dlsym(libhandle,"glGenVertexArraysOES");
glBindVertexArrayFn = (TglBindVertexArrayFn)dlsym(libhandle, "glBindVertexArrayOES");
glDeleteVertexArrayFn = (TglDeleteVertexArrayFn)dlsym(libhandle,"glDeleteVertexArraysOES");
biodranik (Migrated from github.com) commented 2023-04-08 20:44:37 +00:00

These extensions for the non-ES opengl do not have OES suffix, right? Like glGenVertexArrays. Does the current code mean that on Linux we always use OpenGL ES and newer use OpenGL?

These extensions for the non-ES opengl do not have OES suffix, right? Like `glGenVertexArrays`. Does the current code mean that on Linux we _always_ use OpenGL ES and _newer_ use OpenGL?
Ferenc- (Migrated from github.com) reviewed 2023-04-09 10:38:15 +00:00
drape/gl_functions.cpp
@ -260,0 +260,4 @@
LOG(LCRITICAL, ("Failed to open libGL.so.1:", dlerror()));
glGenVertexArraysFn = (TglGenVertexArraysFn)dlsym(libhandle,"glGenVertexArraysOES");
glBindVertexArrayFn = (TglBindVertexArrayFn)dlsym(libhandle, "glBindVertexArrayOES");
glDeleteVertexArrayFn = (TglDeleteVertexArrayFn)dlsym(libhandle,"glDeleteVertexArraysOES");
Ferenc- (Migrated from github.com) commented 2023-04-09 10:38:14 +00:00

This branch and these function pointers are for ES 2.0 only.
The code that uses the functions without the OES suffix,
that is shared between multiple platforms, and desktop GL and ES 3.0 etc is between line 294 and 301.
So no it does not mean that we never use OpenGL.

This branch and these function pointers are for ES 2.0 only. The code that uses the functions without the OES suffix, that is shared between multiple platforms, and desktop GL and ES 3.0 etc is between line 294 and 301. So no it does not mean that we never use OpenGL.
Ferenc- (Migrated from github.com) reviewed 2023-04-09 10:44:17 +00:00
drape/gl_functions.cpp
Ferenc- (Migrated from github.com) commented 2023-04-09 10:44:17 +00:00
  1. If the execution reaches this point, then it is very unlikely that we gain anything from trying to do a lazy resolution, as resolution is going to happen super soon anyway. But if you see value in it then I can change it.
  2. dlclose would decrement the reference counter to the .so, and would potentially unload functions that we will have to keep using till the end of the application's runtime.
1. If the execution reaches this point, then it is very unlikely that we gain anything from trying to do a lazy resolution, as resolution is going to happen super soon anyway. But if you see value in it then I can change it. 2. `dlclose` would decrement the reference counter to the `.so`, and would potentially unload functions that we will have to keep using till the end of the application's runtime.
biodranik (Migrated from github.com) reviewed 2023-04-09 10:44:51 +00:00
drape/gl_functions.cpp
@ -260,0 +260,4 @@
LOG(LCRITICAL, ("Failed to open libGL.so.1:", dlerror()));
glGenVertexArraysFn = (TglGenVertexArraysFn)dlsym(libhandle,"glGenVertexArraysOES");
glBindVertexArrayFn = (TglBindVertexArrayFn)dlsym(libhandle, "glBindVertexArrayOES");
glDeleteVertexArrayFn = (TglDeleteVertexArrayFn)dlsym(libhandle,"glDeleteVertexArraysOES");
biodranik (Migrated from github.com) commented 2023-04-09 10:44:51 +00:00

I see, my bad. Thanks!

I see, my bad. Thanks!
biodranik (Migrated from github.com) reviewed 2023-04-09 10:54:07 +00:00
drape/gl_functions.cpp
biodranik (Migrated from github.com) commented 2023-04-09 10:54:06 +00:00

  1. Does RTLD_NOW resolve all functions? If yes, then does LAZY resolve only the requested ones? We need only a few of them.

  2. We prefer to properly clean up all requested resources, but it may be complicated in this case. Question: is it possible to use eglGetProcAddress here instead of dlopen?

1. Does RTLD_NOW resolve _all_ functions? If yes, then does LAZY resolve only the requested ones? We need only a few of them. 2. We prefer to properly clean up all requested resources, but it may be complicated in this case. Question: is it possible to use `eglGetProcAddress` here instead of dlopen?
biodranik (Migrated from github.com) reviewed 2023-04-09 10:54:51 +00:00
drape/gl_functions.cpp
biodranik (Migrated from github.com) commented 2023-04-09 10:54:50 +00:00
  1. And another question: did you test all 3 cases here? 1) OpenGL ES 2; 2) OpenGL ES 3; 3) Desktop OpenGL?
3. And another question: did you test all 3 cases here? 1) OpenGL ES 2; 2) OpenGL ES 3; 3) Desktop OpenGL?
biodranik (Migrated from github.com) reviewed 2023-04-09 12:18:03 +00:00
Ferenc- (Migrated from github.com) reviewed 2023-04-09 12:44:41 +00:00
Ferenc- (Migrated from github.com) reviewed 2023-04-09 12:53:36 +00:00
drape/gl_functions.cpp
Ferenc- (Migrated from github.com) commented 2023-04-09 12:53:36 +00:00
  1. In any case, I have chaned it back to LAZY.
1. In any case, I have chaned it back to LAZY.
Ferenc- (Migrated from github.com) reviewed 2023-04-09 12:56:30 +00:00
drape/gl_functions.cpp
Ferenc- (Migrated from github.com) commented 2023-04-09 12:56:30 +00:00
  1. Well, the first versions of this PR used eglGetProcAddress because it linked to EGL, which was also a new dependency, and that (new dependencies) was commented as a source of potential problems and impacts. So what has changed?
2. Well, the first versions of this PR used `eglGetProcAddress` because it linked to EGL, which was also a new dependency, and that (new dependencies) was commented as a source of potential problems and impacts. So what has changed?
biodranik (Migrated from github.com) reviewed 2023-04-09 13:25:53 +00:00
drape/gl_functions.cpp
biodranik (Migrated from github.com) commented 2023-04-09 13:25:53 +00:00

Now I remember, thanks for the reminder. I've re-read the comments, and looks like I was wrong, and confused EGL with GLES 🤦 that time. The platform-correct way likely would be to link EGL on Linux and use its methods where necessary. But the existing approach doesn't create additional build dependency so it also makes good sense.
Maybe a comment mentioning that an "appropriate" way to get these addresses would be to use eglGetProcAddress and EGL would be enough ATM.

Now I remember, thanks for the reminder. I've re-read the comments, and looks like I was wrong, and confused EGL with GLES 🤦 that time. The platform-correct way likely would be to link EGL on Linux and use its methods where necessary. But the existing approach doesn't create additional build dependency so it also makes good sense. Maybe a comment mentioning that an "appropriate" way to get these addresses would be to use eglGetProcAddress and EGL would be enough ATM.
vng (Migrated from github.com) approved these changes 2023-04-11 02:07:11 +00:00
bam80 (Migrated from github.com) reviewed 2023-07-06 08:39:57 +00:00
qt/qt_common/map_widget.cpp
@ -340,3 +394,4 @@
m_contextFactory.reset(new QtOGLContextFactory(context()));
emit BeforeEngineCreation();
bam80 (Migrated from github.com) commented 2023-07-06 08:39:57 +00:00

IIUIC, it means LibGLES context is currently used for GLES2 only, and for GLES3 - LibGL context is used.

IIUIC, it means LibGLES context is currently used for GLES2 only, and for GLES3 - LibGL context is used.
Ferenc- (Migrated from github.com) reviewed 2023-07-06 10:08:57 +00:00
qt/qt_common/map_widget.cpp
@ -340,3 +394,4 @@
m_contextFactory.reset(new QtOGLContextFactory(context()));
emit BeforeEngineCreation();
Ferenc- (Migrated from github.com) commented 2023-07-06 10:08:56 +00:00

@bam80
Not exactly, it's more complicated than that.
The current Qt code path in OMaps (where the relevant code path is ancient), for some inexplicable reason, uses the m_apiOpenGLES3 boolean to switch between two shader codes.

  1. The first comes without any #version directive and therefore is compatible with OpenGL 2.0 and OpenGL ES 2.0
  2. The second comes with #version 150 core and therefore is only compatible with desktop OpenGL 3.2 and higher, so no ES.

So if
m_apiOpenGLES3 is true -> Desktop! OpenGL 3.2 and higher is needed
m_apiOpenGLES3 is false -> Either OpenGL ES 2.0 or desktop OpenGL 2.0

In the past I made an attempt to clean up this ancient mess, as it seemed easy.
The idea is to create a new ApiVersion here so we can finally separate GLES3 from GL3, and use separate shader codes in Qt for GLES2, GLES3 and GL3, and forget this m_apiOpenGLES3 mess.

Now as easy it seemed in theory, in practice have faced some inexplicable shader problems resulting in a completely black map. Nowadays I don't have time to play with that, but I am still confident that this would be the right way forward.

@bam80 Not exactly, it's more complicated than that. The current Qt code path in OMaps (where the relevant code path is ancient), for some inexplicable reason, uses the `m_apiOpenGLES3` boolean to [switch between two shader codes.](https://github.com/organicmaps/organicmaps/blob/master/qt/qt_common/map_widget.cpp#L203) 1. The first comes without any `#version` directive and therefore is compatible with `OpenGL 2.0` and `OpenGL ES 2.0` 2. The second comes with `#version 150 core` and therefore is only compatible with desktop `OpenGL 3.2` and higher, so no ES. So if `m_apiOpenGLES3` is `true` -> Desktop! `OpenGL 3.2` and higher is needed `m_apiOpenGLES3` is `false` -> Either `OpenGL ES 2.0` or desktop `OpenGL 2.0` In the past I made an attempt to clean up this ancient mess, as it seemed easy. The idea is to create a new `ApiVersion` [here](https://github.com/organicmaps/organicmaps/blob/201ab8b0e9bba42631454b2d030290bc0f39280b/drape/drape_global.hpp#L19) so we can finally separate `GLES3` from `GL3`, and use separate shader codes in Qt for `GLES2`, `GLES3` and `GL3`, and forget this `m_apiOpenGLES3` mess. Now as easy it seemed in theory, in practice have faced some inexplicable shader problems resulting in a completely black map. Nowadays I don't have time to play with that, but I am still confident that this would be the right way forward.
biodranik (Migrated from github.com) reviewed 2023-07-06 15:58:11 +00:00
qt/qt_common/map_widget.cpp
@ -340,3 +394,4 @@
m_contextFactory.reset(new QtOGLContextFactory(context()));
emit BeforeEngineCreation();
biodranik (Migrated from github.com) commented 2023-07-06 15:58:11 +00:00

@Ferenc- thanks for the details!

  1. Why desktop OpenGL 3.2 and higher are needed if m_apiOpenGLES3 == true? ES is a subset of the desktop GL, so it should just work, no?
  2. Historically, we were targeting only mobile renderers, that's why desktop variants are not present in the enum. Introducing a separate desktop GL branch may be a good idea in the longer term when we have more resources and desktop versions become more popular amongst users. But now it would be much easier to support only mobile ES versions + Vulkan and Metal. What are the cons of this approach in a Linux context?
@Ferenc- thanks for the details! 1. Why desktop OpenGL 3.2 and higher are needed if `m_apiOpenGLES3 == true`? ES is a subset of the desktop GL, so it should just work, no? 2. Historically, we were targeting only mobile renderers, that's why desktop variants are not present in the enum. Introducing a separate desktop GL branch may be a good idea in the longer term when we have more resources and desktop versions become more popular amongst users. But now it would be much easier to support only mobile ES versions + Vulkan and Metal. What are the cons of this approach in a Linux context?
Ferenc- (Migrated from github.com) reviewed 2023-07-07 08:51:47 +00:00
qt/qt_common/map_widget.cpp
@ -340,3 +394,4 @@
m_contextFactory.reset(new QtOGLContextFactory(context()));
emit BeforeEngineCreation();
Ferenc- (Migrated from github.com) commented 2023-07-07 08:51:47 +00:00

ES is a subset of the desktop GL, so it should just work, no?

The subset (ES) is smaller than the superset (desktop), the current shader code, that we have on the m_apiOpenGLES3 == true path (is for desktop GL not for ES!!!) and is using the superset, ergo the subset is not enough.

Historically, we were targeting only mobile renderers, that's why desktop variants are not present in the enum.

That part makes perfect sense, where I don't see the logic anymore is, where the shader code is written with #version 150 core which requires desktop GL, and likely won't compile on mobile renderers.

What are the cons of this approach in a Linux context?

Pros:
Vulkan would make things easier and more maintainable, as well as future proof.

Cons:
Well it takes some effort to implement the change.
And even though OpenGL ES 3.0 is a subset of the currently required superset of desktop OpenGL 3.2.
Simply because OpenGL ES 3.0 is actually chronologically, 7 years newer than OpenGL 3.2.
It can happen, that some older desktop GPUs that don't support Vulkan, but fall into that seven years,
in which OpenGL 3.2 existed but OpenGL ES 3.0 didn't.
And when they try to compile the shader code explicitly written to OpenGL ES 3.0 with #version 300 es,
then this tiny little unknown es at the end is coming from the future, and prevents the shader code from compiling.
But this is a cornercase, and there could be workarounds for it anyway.

So in general I very much like this proposition, and I think the project should go for it.

> ES is a subset of the desktop GL, so it should just work, no? The subset (ES) is smaller than the superset (desktop), the current shader code, that we have on the `m_apiOpenGLES3 == true` path (is for desktop GL not for ES!!!) and is using the superset, ergo the subset is not enough. > Historically, we were targeting only mobile renderers, that's why desktop variants are not present in the enum. That part makes perfect sense, where I don't see the logic anymore is, where the shader code is written with `#version 150 core` which requires desktop GL, and likely won't compile on mobile renderers. > What are the cons of this approach in a Linux context? **Pros:** Vulkan would make things easier and more maintainable, as well as future proof. **Cons:** Well it takes some effort to implement the change. And even though `OpenGL ES 3.0` is a subset of the currently required superset of desktop `OpenGL 3.2`. Simply because `OpenGL ES 3.0` is actually chronologically, 7 years newer than `OpenGL 3.2`. It can happen, that some older desktop GPUs that don't support Vulkan, but fall into that seven years, in which `OpenGL 3.2` existed but `OpenGL ES 3.0` didn't. And when they try to compile the shader code explicitly written to `OpenGL ES 3.0` with `#version 300 es`, then this tiny little unknown `es` at the end is coming from the future, and prevents the shader code from compiling. But this is a cornercase, and there could be workarounds for it anyway. So in general I very much like this proposition, and I think the project should go for it.
bam80 (Migrated from github.com) reviewed 2023-07-07 10:34:34 +00:00
qt/qt_common/map_widget.cpp
@ -340,3 +394,4 @@
m_contextFactory.reset(new QtOGLContextFactory(context()));
emit BeforeEngineCreation();
bam80 (Migrated from github.com) commented 2023-07-07 10:34:34 +00:00

Why desktop OpenGL 3.2 and higher are needed if m_apiOpenGLES3 == true? ES is a subset of the desktop GL, so it should just work, no?

As I can see, it's actually GLSL version which doesn't work, not GL one.
But GL version implies GLSL one by spec, even if it's artificially.
So we can enforce GL version thus influencing GLSL one implicitly, providing fallback otherwise. That would consists the fix.

> Why desktop OpenGL 3.2 and higher are needed if m_apiOpenGLES3 == true? ES is a subset of the desktop GL, so it should just work, no? As I can see, it's actually GLSL version which doesn't work, not GL one. But GL version implies GLSL one by spec, even if it's artificially. So we can enforce GL version thus influencing GLSL one implicitly, providing fallback otherwise. That would consists the fix.
biodranik (Migrated from github.com) reviewed 2023-07-07 23:46:26 +00:00
qt/qt_common/map_widget.cpp
@ -340,3 +394,4 @@
m_contextFactory.reset(new QtOGLContextFactory(context()));
emit BeforeEngineCreation();
biodranik (Migrated from github.com) commented 2023-07-07 23:46:26 +00:00

@Ferenc- m_apiOpenGLES3 == true path works perfectly on all androids now. You can force it by typing ?gl in the search and restarting OM, and return back to vulkan by typing ?vulkan.

For example, from a recent log:

07-06 19:15:26.043 (Thread-17) I/OMcore: drape/support_manager.cpp:35 Init(): Renderer = Mali-G71 | Api = OpenGLES3 | Version = OpenGL ES 3.2 v1.r16p0-01rel0.###other-sha0123456789ABCDEF0###
@Ferenc- `m_apiOpenGLES3 == true` path works perfectly on all androids now. You can force it by typing `?gl` in the search and restarting OM, and return back to vulkan by typing `?vulkan`. For example, from a recent log: ``` 07-06 19:15:26.043 (Thread-17) I/OMcore: drape/support_manager.cpp:35 Init(): Renderer = Mali-G71 | Api = OpenGLES3 | Version = OpenGL ES 3.2 v1.r16p0-01rel0.###other-sha0123456789ABCDEF0### ```
bam80 (Migrated from github.com) reviewed 2023-07-14 21:44:30 +00:00
qt/main.cpp
@ -248,3 +263,3 @@
Framework framework(frameworkParams);
qt::MainWindow w(framework, apiOpenGLES3, std::move(screenshotParams),
qt::MainWindow w(framework, std::move(screenshotParams),
bam80 (Migrated from github.com) commented 2023-07-14 21:44:29 +00:00

@Ferenc- sorry if it was discussed earlier, could you expand the problem to me?
What versions get applied by default without this patch, for XCB and Cocoa?

@Ferenc- sorry if it was discussed earlier, could you expand the problem to me? What versions get applied by default without this patch, for XCB and Cocoa?
This repo is archived. You cannot comment on pull requests.
Reviewers
No reviewers
biodranik
vng
Labels
Clear labels   
Accessibility

issues affecting users with specific needs

  
Accessibility

issues affecting users with specific needs

  
Address

Issues with addresses, post codes, city/town/village borders

  
Address

Issues with addresses, post codes, city/town/village borders

  
Android

Android development

  
Android

Android development

  
Android Auto

Android Auto

  
Android Auto

Android Auto

  
Android Automotive (AAOS)

   
Android Automotive (AAOS)

   
API

Short Links, deep links and url schemes, including ge0 and om://

  
API

Short Links, deep links and url schemes, including ge0 and om://

  
AppGallery

Huawei AppGallery

  
AppGallery

Huawei AppGallery

  
AppStore

Apple AppStore

  
AppStore

Apple AppStore

  
Battery and Performance

Performance and battery consumption-related issues and optimizations

  
Battery and Performance

Performance and battery consumption-related issues and optimizations

  
Blocker

An issue blocking the release

  
Blocker

An issue blocking the release

  
Bookmarks and Tracks

Bookmarks, imported tracks, and KML, KMZ, KMB, GPX, GPZ import or export

  
Bookmarks and Tracks

Bookmarks, imported tracks, and KML, KMZ, KMB, GPX, GPZ import or export

  
Borders

Country borders and polygons

  
Borders

Country borders and polygons

  
Bug

Something isn't working

  
Bug

Something isn't working

  
Build

Compilation issue

  
Build

Compilation issue

  
CarPlay

CarPlay Integration

  
CarPlay

CarPlay Integration

  
Classificator

Classify object by types

  
Classificator

Classify object by types

  
Community

Community relations

  
Community

Community relations

  
Core

Cross-platform C++ libraries with a core functionality

  
Core

Cross-platform C++ libraries with a core functionality

  
CrashReports

Crash Reporting

  
CrashReports

Crash Reporting

  
Cycling

Bike, cycle, biking and related

  
Cycling

Bike, cycle, biking and related

  
Desktop

Desktop application

  
Desktop

Desktop application

  
DevEx

Developer Experience

  
DevEx

Developer Experience

  
DevOps

Automation and pipelines

  
DevOps

Automation and pipelines

  
dev_sandbox

A desktop app to debug rendering

  
dev_sandbox

A desktop app to debug rendering

  
Directions

Issues with next turns, labels, streets, voice instructions

  
Directions

Issues with next turns, labels, streets, voice instructions

  
Documentation

Improvements or additions to documentation

  
Documentation

Improvements or additions to documentation

  
Downloader

Map dowloader

  
Downloader

Map dowloader

  
Drape

Drape OpenGL, Vulkan and Metal graphics rendering engine

  
Drape

Drape OpenGL, Vulkan and Metal graphics rendering engine

  
Driving

Drive a car related issues

  
Driving

Drive a car related issues

  
Duplicate

This issue or pull request already exists

  
Duplicate

This issue or pull request already exists

  
Editor

OSM Editor

  
Editor

OSM Editor

  
Elevation

Isolines and altitude

  
Elevation

Isolines and altitude

  
Enhancement

New feature or request, an improvement of some existing feature

  
Enhancement

New feature or request, an improvement of some existing feature

  
Epic

A large body work

  
Epic

A large body work

  
External Map Datasets

TIGER, Ordnance Survey, ASTER, SRTM, etc.

  
External Map Datasets

TIGER, Ordnance Survey, ASTER, SRTM, etc.

  
F-Droid

F-Droid build and distribution

  
F-Droid

F-Droid build and distribution

  
Fonts

Font and text rendering

  
Fonts

Font and text rendering

  
Frequently User Reported

Most annoying UX and other issues for users reported by many people

  
Frequently User Reported

Most annoying UX and other issues for users reported by many people

  
Fund

   
Fund

   
Generator

Related to generator tool.

  
Generator

Related to generator tool.

  
Good first issue

Good for newcomers

  
Good first issue

Good for newcomers

  
Google Play

Google Play

  
Google Play

Google Play

  
GPS

Location and positioning issues

  
GPS

Location and positioning issues

  
GSoC

Ideas for Google Summer of Code

  
GSoC

Ideas for Google Summer of Code

  
iCloud

   
iCloud

   
Icons

Map and app icons

  
Icons

Map and app icons

  
iOS

iOS development

  
iOS

iOS development

  
Legal

Legal aspects

  
Legal

Legal aspects

  
Linux Desktop

Ubuntu, Gentoo, Red Hat, Debian, Cent OS, Fedora, etc.

  
Linux Desktop

Ubuntu, Gentoo, Red Hat, Debian, Cent OS, Fedora, etc.

  
Linux packaging

   
Linux packaging

   
Linux Phone

LibreM, PinePhone, Wayland and other mobile Linux versions

  
Linux Phone

LibreM, PinePhone, Wayland and other mobile Linux versions

  
Mac OS

Desktop issues related to Mac OS X, OM running on M1, M2, M3 Apple silicon, or Qt version

  
Mac OS

Desktop issues related to Mac OS X, OM running on M1, M2, M3 Apple silicon, or Qt version

  
Map Data

OpenStreetMap data related issues

  
Map Data

OpenStreetMap data related issues

  
Metro

Subway, Tube, light-rail public transport issues

  
Metro

Subway, Tube, light-rail public transport issues

  
Navigation

Issues related to the active Navigator / Navigation mode

  
Navigation

Issues related to the active Navigator / Navigation mode

  
Need Feedback

Further information is requested

  
Need Feedback

Further information is requested

  
Night Mode

Night or dark mode feature

  
Night Mode

Night or dark mode feature

  
NLnet 2024-06-281

Tasks related to NLnet 2024-06-281 project.

  
NLnet 2024-06-281

Tasks related to NLnet 2024-06-281 project.

  
No Feature Parity

A platform needs this to be implemented to have feature parity with other platforms

  
No Feature Parity

A platform needs this to be implemented to have feature parity with other platforms

  
Opening Hours

Business Hours, off time, holidays, seasonal

  
Opening Hours

Business Hours, off time, holidays, seasonal

  
Outdoors

Hiking, mtb, 4x4...

  
Outdoors

Hiking, mtb, 4x4...

  
POI Info

Feature metadata, OSM tags that are displayed in Place Page

  
POI Info

Feature metadata, OSM tags that are displayed in Place Page

  
Privacy

Privacy

  
Privacy

Privacy

  
Public Transport

Bus, trolleybus, train, tram, metro, subway, light rail

  
Public Transport

Bus, trolleybus, train, tram, metro, subway, light rail

  
Raw Idea

An idea that needs to be elaborated

  
Raw Idea

An idea that needs to be elaborated

  
Refactoring

   
Refactoring

   
Regional

This issue is different depending on the country or region

  
Regional

This issue is different depending on the country or region

  
Regression

A regression bug

  
Regression

A regression bug

  
Releases

Meta-tickets for tracking the release procedure

  
Releases

Meta-tickets for tracking the release procedure

  
RoboTest

Robo / Monkey Auto Tests

  
RoboTest

Robo / Monkey Auto Tests

  
Route Planning

Preview and plan your track

  
Route Planning

Preview and plan your track

  
Routing

Route building issues, e.g. valid route, valid ETA

  
Routing

Route building issues, e.g. valid route, valid ETA

  
Ruler

Helicopter routing, "as a crow flies"

  
Ruler

Helicopter routing, "as a crow flies"

  
Search

Search

  
Search

Search

  
Security

Security improvements

  
Security

Security improvements

  
Styles

Map drawing styles

  
Styles

Map drawing styles

  
Tests

   
Tests

   
Track Recording

Track Recording

  
Track Recording

Track Recording

  
Translations

Localization and translations issues

  
Translations

Localization and translations issues

  
TTS

Text-to-Speech

  
TTS

Text-to-Speech

  
UI

User interface issues

  
UI

User interface issues

  
UX

User eXperience, an issue with usability

  
UX

User eXperience, an issue with usability

  
Walk Navigation

Foot pedestrian mode

  
Walk Navigation

Foot pedestrian mode

  
Watches

Android Watches or Apple Watches support

  
Watches

Android Watches or Apple Watches support

  
Web

https://organicmaps.app/

  
Web

https://organicmaps.app/

  
Wikipedia

Issues related to Wiki articles embedded in OM data files.

  
Wikipedia

Issues related to Wiki articles embedded in OM data files.

  
Windows

Windows

  
Windows

Windows

  
Won't fix

Unfeasible or impossible to resolve

  
Won't fix

Unfeasible or impossible to resolve

  
World Map

Issues about improving the base map of the whole world that is bundled with the app

  
World Map

Issues about improving the base map of the whole world that is bundled with the app

No labels
Accessibility
Accessibility
Address
Address
Android
Android
Android Auto
Android Auto
Android Automotive (AAOS)
Android Automotive (AAOS)
API
API
AppGallery
AppGallery
AppStore
AppStore
Battery and Performance
Battery and Performance
Blocker
Blocker
Bookmarks and Tracks
Bookmarks and Tracks
Borders
Borders
Bug
Bug
Build
Build
CarPlay
CarPlay
Classificator
Classificator
Community
Community
Core
Core
CrashReports
CrashReports
Cycling
Cycling
Desktop
Desktop
DevEx
DevEx
DevOps
DevOps
dev_sandbox
dev_sandbox
Directions
Directions
Documentation
Documentation
Downloader
Downloader
Drape
Drape
Driving
Driving
Duplicate
Duplicate
Editor
Editor
Elevation
Elevation
Enhancement
Enhancement
Epic
Epic
External Map Datasets
External Map Datasets
F-Droid
F-Droid
Fonts
Fonts
Frequently User Reported
Frequently User Reported
Fund
Fund
Generator
Generator
Good first issue
Good first issue
Google Play
Google Play
GPS
GPS
GSoC
GSoC
iCloud
iCloud
Icons
Icons
iOS
iOS
Legal
Legal
Linux Desktop
Linux Desktop
Linux packaging
Linux packaging
Linux Phone
Linux Phone
Mac OS
Mac OS
Map Data
Map Data
Metro
Metro
Navigation
Navigation
Need Feedback
Need Feedback
Night Mode
Night Mode
NLnet 2024-06-281
NLnet 2024-06-281
No Feature Parity
No Feature Parity
Opening Hours
Opening Hours
Outdoors
Outdoors
POI Info
POI Info
Privacy
Privacy
Public Transport
Public Transport
Raw Idea
Raw Idea
Refactoring
Refactoring
Regional
Regional
Regression
Regression
Releases
Releases
RoboTest
RoboTest
Route Planning
Route Planning
Routing
Routing
Ruler
Ruler
Search
Search
Security
Security
Styles
Styles
Tests
Tests
Track Recording
Track Recording
Translations
Translations
TTS
TTS
UI
UI
UX
UX
Walk Navigation
Walk Navigation
Watches
Watches
Web
Web
Wikipedia
Wikipedia
Windows
Windows
Won't fix
Won't fix
World Map
World Map
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: organicmaps/organicmaps-tmp#2947
No description provided.
Delete branch "wayland_es2"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?