Compile error with latest GLM

[Jean Philippe Eimer]

I need the attached patch to compile master branch against latest GLM library (0.9.9/git).

[Maciej Suminski (orsonmmz)]

Hi Jean Philippe,

Thank you for the patch. Would you tell us more about the error? I wonder why now GLM requires GLM_ENABLE_EXPERIMENTAL definition, as presumably we have not started using any features introduced in 0.9.9.

[Jean Philippe Eimer (phileimer)]

Hi Maciej,

For example, in '/3d-viewer/3d_cache/sg/scenegraph.cpp', I have several
compile errors with 'glm::rotate'.

Without GLM_ENABLE_EXPERIMENTAL, and to avoid error, the code should be
modified.
Line 745, from :
glm::dmat4 rM = glm::rotate( rotation_angle, glm::dvec3( rX, rY, rZ ) );

To :
glm::dmat4 rM = glm::rotate( rM, rotation_angle, glm::dvec3( rX, rY, rZ ) );

There are several other errors like this :
* glm::rotate in '3d-viewer/3d_rendering/3d_render_ogl_legacy/
c3d_render_createscene_ogl_legacy.cpp', line 113

* #include <glm/gtx/transform.hpp>, in
'plugins/3d/vrml/v1/vrml1_node.h', line 36

For this later case, here is the error displayed :

In file included from
/home/phil/src/kicad/plugins/3d/vrml/v1/vrml1_node.h:36:0,
                  from
/home/phil/src/kicad/plugins/3d/vrml/v1/vrml1_base.h:36,
                  from /home/phil/src/kicad/plugins/3d/vrml/vrml.cpp:41:
/usr/include/glm/gtx/transform.hpp:23:3: error: #error "GLM:
GLM_GTX_transform is an experimental extension and may change in the
future. Use #define GLM_ENABLE_EXPERIMENTAL before including it, if you
really want to use it."

For your information, I use the latest gcc 7.3 compiler on Linux.

Do not hesitate to ask for more feedback if needed.

Best regards,
J. Ph.

Le 31/01/2018 à 17:52, Maciej Suminski a écrit :
> Hi Jean Philippe,
>
> Thank you for the patch. Would you tell us more about the error? I
> wonder why now GLM requires GLM_ENABLE_EXPERIMENTAL definition, as
> presumably we have not started using any features introduced in 0.9.9.
>

[Wayne Stambaugh (stambaughw)]

Orson,

The idea that we would commit a patch that enables something marked
experimental this close to a stable release makes me nervous. Please
proceed with caution.

Thanks,

Wayne

On 1/31/2018 11:52 AM, Maciej Suminski wrote:
> Hi Jean Philippe,
>
> Thank you for the patch. Would you tell us more about the error? I
> wonder why now GLM requires GLM_ENABLE_EXPERIMENTAL definition, as
> presumably we have not started using any features introduced in 0.9.9.
>

[Seth Hillbrand (sethh)]

Here's some background. https://github.com/g-truc/glm/blob/master/manual.md#section7_4

The GTX extensions were always experimental but were exposed previously without needing a flag. 0.9.9 requires that the flag be set to access the same features.

[Jean Philippe Eimer (phileimer)]

Hi,

I confirm the GLM patch is not needed when KiCAD is compiled against
stable GLM 0.9.8.5.

However, this stable GLM must be patched (see attachement) to deal with
the new gcc 7.3.
But, it's a GLM issue.

Hence, there are 2 solutions to compile with gcc 7.3 :
* use the attached patch with stable glm 0.9.8.5
* use the latest git GLM, and patch kicad for GLM_ENABLE_EXPERIMENTAL

Regarding the 'experimental' fear, I don't understand why some GLM
function prototypes are experimental in the new GLM, whereas they were
not in stable release.

Regards,
J. Ph.

Le 31/01/2018 à 21:30, Wayne Stambaugh a écrit :
> Orson,
>
> The idea that we would commit a patch that enables something marked
> experimental this close to a stable release makes me nervous. Please
> proceed with caution.
>
> Thanks,
>
> Wayne
>
> On 1/31/2018 11:52 AM, Maciej Suminski wrote:
>> Hi Jean Philippe,
>>
>> Thank you for the patch. Would you tell us more about the error? I
>> wonder why now GLM requires GLM_ENABLE_EXPERIMENTAL definition, as
>> presumably we have not started using any features introduced in 0.9.9.
>>
>

[Maciej Suminski (orsonmmz)]

Jean Philippe,

Would you have some spare time to modify KiCad code so it compiles without the GLM_ENABLE_EXPERIMENTAL flag? I agree with Wayne and would rather remain careful regarding experimental features. There is no guarantee that the flag would not change something else in GLM code, so it is safer to adapt our code.

GLM 0.9.9 is not yet available in Arch Linux, which normally uses bleeding edge packages, therefore I guess we still have some time to address the problem.

[Jean Philippe Eimer (phileimer)]

Here is a tentative patch to avoid GLM_ENABLE_EXPERIMENTAL.
Please test it, as I don't know the Kicad code enough to be comfortable with the checks I did.

[KiCad Janitor (kicad-janitor)]

Fixed in revision e0f7958dbdab89f38bacc6a567e21b38a0926aee
https://git.launchpad.net/kicad/patch/?id=e0f7958dbdab89f38bacc6a567e21b38a0926aee

[Maciej Suminski (orsonmmz)]

Jean Philippe,

Many thanks for the patch, especially you have prepared it so quickly. I verified it and merged it to the master branch.

[Nick Østergaard (nickoe)]

I think this broke the build on fedora26 and fedora 27 which uses glm 0.9.8.4.

/usr/include/glm/detail/type_vec4_simd.inl:156:11: error: 'struct glm::tvec4<float, (glm::precision)5>' has no member named 'data'
/usr/include/glm/detail/type_vec4_simd.inl:156:11: error: 'struct glm::tvec4<float, (glm::precision)5>' has no member named 'data'
    Result.data = _mm_mul_ps(a.data, _mm_rcp_ps(b.data));
    Result.data = _mm_mul_ps(a.data, _mm_rcp_ps(b.data));
           ^~~~
           ^~~~

https://copr.fedorainfracloud.org/coprs/g/kicad/kicad/build/711229/

[Jean Philippe Eimer (phileimer)]

Hi Nick,

Thank you for this feedback.

On my system (which is not Fedora), I can build Kicad with glm-0.9.8.4, glm-0.9.8.5, or glm-0.9.9-master, and I have no error.

Also, the Fedora 26/27 build failure occurs at 'common/draw_panel_gal.cpp' compilation, whereas this file is not affected by the GLM patch.

This Fedora 26/27 problem is not related to the GLM-0.9.9 patch.

After browsing your log, I found that Fedora 27 uses gcc 7.3.
As stated above (message #6), GLM prior to 0.9.9 needs to be patched for gcc 7.3.

[Maciej Suminski (orsonmmz)]

We are discussing with another Fedora user, which says the problem is not necessarily caused by the patch. We wait for more information.

[Jean Philippe Eimer (phileimer)]

Definitively, it's not caused by the patch.
Fedora's GLM must either be patched for gcc 7.3, or updated to 0.9.9.

[Seth Hillbrand (sethh)]

@Jean Philippe-

You should be able to add a #define GLM_FORCE_COMPILER_UNKNOWN to the top of vertex_manager.h to avoid the gcc 7.3 issue instead of patching glm.

[Jean Philippe Eimer (phileimer)]

Thank you Seth for this idea.
I let project managers decide whether they want to modify Kicad code for this.

However, I think Fedora will have to patch GLM, because its source tarball can't be built/installed anymore with gcc 7.3.

[Nick Østergaard (nickoe)]

It looks like the fedora builds are working again. I guess the issue I noticed was in fedora then. Thanks.