[Bug 296141] New: Make digikam compile on ARM

https://bugs.kde.org/show_bug.cgi?id=296141

            Bug ID: 296141
          Severity: normal
           Version: 2.6.0
          Priority: NOR
          Assignee: [hidden email]
           Summary: Make digikam compile on ARM
    Classification: Unclassified
                OS: Linux
          Reporter: [hidden email]
          Hardware: Ubuntu Packages
            Status: NEW
         Component: Portability
           Product: digikam

Created attachment 69659
  --> https://bugs.kde.org/attachment.cgi?id=69659&action=edit
kubuntu_fix_arm.patch

Currently digikam does not compile on ARM, I've attached a patch that fixes
digikam compiles on ARM

--
You are receiving this mail because:
You are the assignee for the bug.
_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

[Gilles Caulier-4]

https://bugs.kde.org/show_bug.cgi?id=296141

Gilles Caulier <[hidden email]> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[hidden email]

--- Comment #1 from Gilles Caulier <[hidden email]> ---
Just by curiosity, on which ARM device you compile digiKam exactly ?

Gilles Caulier

--
You are receiving this mail because:
You are the assignee for the bug.
_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

https://bugs.kde.org/show_bug.cgi?id=296141

--- Comment #2 from Rohan Garg <[hidden email]> ---
Ah, This is a ARM pbuilder that I setup on my laptop, uses qemu-system-arm to
emulate a ARM processor

--
You are receiving this mail because:
You are the assignee for the bug.
_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

[Gilles Caulier-4]

https://bugs.kde.org/show_bug.cgi?id=296141

Gilles Caulier <[hidden email]> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|---                         |FIXED

--- Comment #3 from Gilles Caulier <[hidden email]> ---
Git commit e0a2fcfb9f0bc75084e4bf01905524fd5e29edd6 by Gilles Caulier.
Committed on 16/03/2012 at 13:48.
Pushed by cgilles into branch 'master'.

fix compilation under ARM device. Apply patch #69659

M  +1    -1    libs/database/searchxml.cpp
M  +1    -1    libs/database/searchxml.h
M  +2    -2    libs/widgets/graphicsview/regionframeitem.cpp
M  +1    -1    utilities/searchwindow/searchfields.cpp

http://commits.kde.org/digikam/e0a2fcfb9f0bc75084e4bf01905524fd5e29edd6

--
You are receiving this mail because:
You are the assignee for the bug.
_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

[Francesco Riosa-2]

https://bugs.kde.org/show_bug.cgi?id=296141

Francesco Riosa <[hidden email]> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[hidden email]

--- Comment #4 from Francesco Riosa <[hidden email]> ---
from #1: <<Typedef for double on all platforms except for those using CPUs with
ARM architectures. On ARM-based platforms, qreal is a typedef for float for
performance reasons.>>

according to wikipedia [#2] <<This gives from 6 to 9 significant decimal digits
precision>>

should the precision of the comparison (8) be lowered too?

#1 http://doc.qt.nokia.com/4.7-snapshot/qtglobal.html#qreal-typedef
#2 http://en.wikipedia.org/wiki/Single-precision_floating-point_format

--
You are receiving this mail because:
You are the assignee for the bug.
_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

[Francesco Riosa-2]

https://bugs.kde.org/show_bug.cgi?id=296141

--- Comment #5 from Francesco Riosa <[hidden email]> ---
And _not_ only for ARM, since it's a value stored in database and thus
potentially accessed from other platforms it should be the same for all

--
You are receiving this mail because:
You are the assignee for the bug.
_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

[Ananta Palani]

https://bugs.kde.org/show_bug.cgi?id=296141

Ananta Palani <[hidden email]> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[hidden email]

--- Comment #6 from Ananta Palani <[hidden email]> ---
(In reply to comment #4)

> from #1: <<Typedef for double on all platforms except for those using CPUs
> with ARM architectures. On ARM-based platforms, qreal is a typedef for float
> for performance reasons.>>
>
> according to wikipedia [#2] <<This gives from 6 to 9 significant decimal
> digits precision>>
>
> should the precision of the comparison (8) be lowered too?
>
> #1 http://doc.qt.nokia.com/4.7-snapshot/qtglobal.html#qreal-typedef
> #2 http://en.wikipedia.org/wiki/Single-precision_floating-point_format

Probably should be changed to 6 since that is the default for QString::number()
anyway:

   http://qt-project.org/doc/qt-4.8/qstring.html#number-2

--
You are receiving this mail because:
You are the assignee for the bug.
_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

[Ananta Palani]

https://bugs.kde.org/show_bug.cgi?id=296141

--- Comment #7 from Ananta Palani <[hidden email]> ---
Or is the precision of 8 necessary for some GPS/lat/long coordinates or
something similar?

--
You are receiving this mail because:
You are the assignee for the bug.
_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

[Gilles Caulier-4]

https://bugs.kde.org/show_bug.cgi?id=296141

--- Comment #8 from Gilles Caulier <[hidden email]> ---
Francesco,

The float precision is important of course, especialy for geolocation data. Do
you know a way to fix qReal usability problem with DB ?

Gilles Caulier

--
You are receiving this mail because:
You are the assignee for the bug.
_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

[Francesco Riosa-2]

https://bugs.kde.org/show_bug.cgi?id=296141

--- Comment #9 from Francesco Riosa <[hidden email]> ---
(In reply to comment #8)
> The float precision is important of course, especialy for geolocation data.
> Do you know a way to fix qReal usability problem with DB ?

All the following are guesstimates, take them with a grain of salt:
< cortex-A7 generally don't do floating point (FP)
cortex A7 has VFP extension which should do single and double FP
>= cortex A8 have NEON extensions (A9 optional) which are also suitable to vector processing

So we must assume that ARM _only_support single FP precision. This is baad
because it could taint a database for every other architecture.

Assuming we are not using those values for calculations, they could be stored
the values as strings instead of number, but this has to be done everywere from
EXIV(?) to the insertion in the db.

So really no good ideas here.

Rohan Garg;
   since you have acces to ARM do you have any idea on how a double could be
managed?
What was the actual error the compiler gave you?

Links:
http://en.wikipedia.org/wiki/ARM_architecture
http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.faqs/ka9347.html

--
You are receiving this mail because:
You are the assignee for the bug.
_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

https://bugs.kde.org/show_bug.cgi?id=296141

--- Comment #10 from Rohan Garg <[hidden email]> ---
After a discussion on #ubuntu-arm it was concluded that
a) ARMv7 and above has good double support, hence qreal should typedef to
double on ARMv7 architectures and above ( Should go into Qt 5 )

b) For architechtures below v7, only single point precision is available, and
double precision is not possible.

Build logs can be found here :
https://launchpad.net/ubuntu/+source/digikam/4:2.5.0-1ubuntu1/+build/3215911
https://launchpad.net/ubuntu/+source/digikam/4:2.5.0-1ubuntu1/+build/3215910

--
You are receiving this mail because:
You are the assignee for the bug.
_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

[Francesco Riosa-2]

https://bugs.kde.org/show_bug.cgi?id=296141

Francesco Riosa <[hidden email]> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |REOPENED
         Resolution|FIXED                       |---

--- Comment #11 from Francesco Riosa <[hidden email]> ---
(In reply to comment #10)
> After a discussion on #ubuntu-arm it was concluded that
> a) ARMv7 and above has good double support, hence qreal should typedef to
> double on ARMv7 architectures and above ( Should go into Qt 5 )

sorry, this has already discussed (in a _very_ long thread) and rejected

Decide whether qreal should be double on all platforms.
https://bugreports.qt-project.org/browse/QTBUG-23758
== We'll keep it as it for Qt 5. double on desktop and float on embedded
devices.


[Development] Changing qreal to a float
http://www.mail-archive.com/development@.../msg01969.html


> b) For architechtures below v7, only single point precision is available,
> and double precision is not possible.
>
> Build logs can be found here :
> https://launchpad.net/ubuntu/+source/digikam/4:2.5.0-1ubuntu1/+build/3215911
> https://launchpad.net/ubuntu/+source/digikam/4:2.5.0-1ubuntu1/+build/3215910

thanks,
  I see froom the logs that the problem is at link time, i.e. for example
qBound() accepting only float on ARM and double on "desktop"

Since we are not using these values to do intensive calculations, and since ARM
seem to have a "double" type, just not hardware support for it I would suggest
the following:

- in regionframeitem.cpp use whatever, it's not precision dependant (but check
it compile ;)
- searchxml.* searchfields.* use double
  convert all usage of qBound, qMax, qMin to something like:
  qreal x = qBound(qreal(min), v, qreal(max));
  double x = std::max<double>(min, std::min(value, max))

Rohan Garg, do you feel brave enough to issue another patch following these
guidelines?
It would simplify things a lot.

Thanks in advance

--
You are receiving this mail because:
You are the assignee for the bug.
_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

https://bugs.kde.org/show_bug.cgi?id=296141

--- Comment #12 from Rohan Garg <[hidden email]> ---
Hi Francesco
Sorry but I'm already spread pretty thin over multiple projects, however I can
help test any proposed patches using a ARM builder.

--
You are receiving this mail because:
You are the assignee for the bug.
_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

[Francesco Riosa-2]

https://bugs.kde.org/show_bug.cgi?id=296141

Francesco Riosa <[hidden email]> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|REOPENED                    |RESOLVED
         Resolution|---                         |FIXED

--- Comment #13 from Francesco Riosa <[hidden email]> ---
Git commit 42b51fc3af4c724bd3b88ed6cd602ea49baf7f18 by Francesco Riosa.
Committed on 20/03/2012 at 16:15.
Pushed by riosa into branch 'master'.

fix compilation under ARM device.

qReal translate to different types on different architectures.
Consensus is that the current state of affairs will be kept for qt 5.0
too [1].

Luckily the function affected "SearchFieldRangeInt" is only used to
store exposure times [2] where 6 precision digit suffice.

So we add functions to write <float> and cast to <qReal> where needed
and be done with that.

[1] https://bugreports.qt-project.org/browse/QTBUG-23758
[2] ChangeLog 2008-07-07 For use with the exposuretime field ...

P.S. CustomStepsDoubleSpinBox could be converted to
     CustomStepsFloatSpinBox but seem not worth the effort.
     Marcel hope you're ok with this

CCMAIL: [hidden email]

M  +14   -0    libs/database/searchxml.cpp
M  +2    -0    libs/database/searchxml.h
M  +2    -2    libs/widgets/graphicsview/regionframeitem.cpp
M  +1    -1    utilities/searchwindow/searchfields.cpp

http://commits.kde.org/digikam/42b51fc3af4c724bd3b88ed6cd602ea49baf7f18

--
You are receiving this mail because:
You are the assignee for the bug.
_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

[Francesco Riosa-2]

https://bugs.kde.org/show_bug.cgi?id=296141

--- Comment #14 from Francesco Riosa <[hidden email]> ---
(In reply to comment #12)
> Hi Francesco
> Sorry but I'm already spread pretty thin over multiple projects, however I
> can help test any proposed patches using a ARM builder.

Ok, I've reverted the previous commit and replaced with a different one, result
should be the same.
Testing on arm would be apreciated.

--
You are receiving this mail because:
You are the assignee for the bug.
_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

https://bugs.kde.org/show_bug.cgi?id=296141

--- Comment #15 from Rohan Garg <[hidden email]> ---
Currently compiling digikam on a pandaboard, will take a couple of hours ( or
minutes if the build fails ). Will post the entire log when it's done/fails

--
You are receiving this mail because:
You are the assignee for the bug.
_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

https://bugs.kde.org/show_bug.cgi?id=296141

--- Comment #16 from Rohan Garg <[hidden email]> ---
Tested a compile and it works!
Thanks Francesco!

--
You are receiving this mail because:
You are the assignee for the bug.
_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

[Gilles Caulier-4]

https://bugs.kde.org/show_bug.cgi?id=296141

Gilles Caulier <[hidden email]> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Version Fixed In|                            |2.6.0

--
You are receiving this mail because:
You are the assignee for the bug.
_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

[bugzilla_noreply]

https://bugs.kde.org/show_bug.cgi?id=296141

[hidden email] changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
          Component|Portability                 |Bundle-Tarball

--
You are receiving this mail because:
You are the assignee for the bug.