impagina / core

An experimental layout engine based on Scribus code.
GNU General Public License v2.0
5 stars 0 forks source link

should we use qreal or double? #23

Open aoloe opened 10 years ago

aoloe commented 10 years ago

qreal is useful on arm... and i think that smal arm server could be a target for the command line scribus...

also, most Qt functions work with qreals.

but there are also issues, like the missing QString::toQReal() which we are faking with the TOQREAL() macro.

aoloe commented 10 years ago

this irc discussion is about fixing 1.4.2 to work on arm. there are a few interesting facts:

<williamhbell> On the subject of building for ARM, why did you choose to ignore the types used within qt4 ? <williamhbell> qreal? <MrB> Scribus was developed first with Qt2.. and then many years with Qt3.. so significant parts of the code were already using doubles <williamhbell> Is there anything dangerous abouting using qreal i.e. float for ARM ? <williamhbell> ARM <williamhbell> Qt4 uses qreal <williamhbell> which is a typede <jghali> williamhbell: our main rendering engine which is based on cairo also use doubles <williamhbell> typedef <williamhbell> All of the Qt4 functions use <qreal> <williamhbell> template instantiations within the header file <MrB> but cairo does not <williamhbell> Does it use QVectors ? <MrB> no its pure c++ <jghali> cairo doesn't know anything about qt <williamhbell> Fine <williamhbell> So at some point you have to fill a QVector <williamhbell> or a QList <williamhbell> these are QVector<qreal> and QList<qreal> in the qt4 header files <jghali> yes, but that won't work with cairo <jghali> cairo requires double <williamhbell> This means you are copying the Cairo doubles into QVector<double> ? <williamhbell> since Cairo is pure C++ <williamhbell> ? <williamhbell> As long as all of the Qt calls are with <qreal> there is no compilation issue <williamhbell> Cairo could then keep double <jghali> no we pass QVector<double> underlying array directly to cairo which is written in C not C++ <williamhbell> No problem <williamhbell> ...thinking... <jghali> if we use qreal that won't work on arm, unless we instantiate another double array which will kill performance <williamhbell> Well at the moment it will not compile at all <williamhbell> That is 1.4.3 does not compile <williamhbell> in 1.4.1 the qreal problems were smaller <williamhbell> We cannot hack qt4 just for Scribus <williamhbell> The problem is in <williamhbell> qglobal.h <williamhbell> #elif defined(QT_NO_FPU) || defined(QT_ARCH_ARM) || defined(QT_ARCH_WINDOWSCE) || defined(QT_ARCH_SYMBIAN) <williamhbell> typedef float qreal; <jghali> yeap i known, but cairo requires doubles and we can't change that either <williamhbell> Sure, I get the point about Cairo <williamhbell> So, if I can come up with a way to copy vector<float> to vector<double> when qreal is float with very little overhead everyone is happy? <williamhbell> Or is there some other issue? <jghali> no, this is the main issue <williamhbell> The exponent and mantissa could both be unions with the longer version. <MrB> williamhbell: what.. erm.. org(?) are you from? raspbian? or qt? or? <williamhbell> Raspbian <MrB> ok <williamhbell> Have you tried to compile Scribus 1.4.3 on Raspbian using a Raspberry Pi? <williamhbell> It takes about 6hrs before it exits due to the qreal problem. <williamhbell> I will go away and try to cook up a very fast double to float conversion then? <williamhbell> The qreal as float for ARM is still in Qt5 btw <MrB> not surprised <MrB> so for Scribus 1.5.0 we can apply any changes too, as it is now qt5 only <williamhbell> The qreal issue is in Qt4 and Qt5 <williamhbell> How about using some ARM assembler if it is ARM? <williamhbell> http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0491c/BABGABJH.html <mrscribe> Title: ARM Information Center (at infocenter.arm.com) <MrB> how would we use that ? <jghali> i doubt the performance problem will come from float to double conversion, my guess is the bottleneck will be the memory allocation/desallocation of the temporary arrays <williamhbell> The ARM assember is supposed to do the conversion without moving the data out of the NEON registers <williamhbell> Can anyone point me at where the QVector<double> is given to Cairo? <williamhbell> or the internal double array that is? <jghali> all cairo stuff happens in scpainter.cpp <williamhbell> Thanks <williamhbell> The debian people had a patch for that and Scribus 1.4.1 <williamhbell> but the problems spread outside scpainter.cpp in 1.4.2 <williamhbell> http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=707882 <mrscribe> Title: #707882 - scribus FTBFS on armel and armhf, qreal vs double problems - Debian Bug report logs (at bugs.debian.org) <williamhbell> It looks even worse in 1.4.3 <jghali> it seems that patch can be applied without any problem <williamhbell> Plugwash was not sure <jghali> williamhbell, unfortunately that patch is insufficient, it breaks the widget on non arm architecture, a much bigger patch would be required <jghali> btw Qt makes the difference between qreal and double in signal/slots, even non non arm architecture <jghali> so such line is not correct: <jghali> connect(Rechteck, SIGNAL(FormSel(int, int, double )), this, SLOT(SelShape(int, int, double ))) <jghali> as the FormSel signal is l(int, int, qreal *) <jghali> then when you try to fix it, the changes required explode <williamhbell> So what is the plan with respect to ARM <williamhbell> should we just say ARM is not supported? <williamhbell> and Scribus cannot be compiled for ARM from 1.4.2 onwards? <williamhbell> Or should I try to make a fast QVector<double> to QVector<float> routine? <jghali> it looks like such a conversion routine will not be sufficient without extensive changes... <jghali> and we never claimed support for ARM architecture <jghali> my opinion would be to say "ARM is not supported". <williamhbell> We have been writing creating the MagPi magazine on ARM using Scribus.