Page MenuHomePhabricator

Eina_Matrix : Use math header for cosf and sinf of rotate function.

Authored by jsuya on Oct 24 2019, 4:37 AM.



The local cos and sin functions differ from
the math header cos and sin functions by result values
The 4th decimal place is different.
Computing large numbers can cause errors.

Test Plan

degree 30
math : rad : 0.523599 cos: 0.866025 sin : 0.500000
local : rad : 0.523599, cos : 0.866667 sin : 0.500000

degree 45
math : rad : 0.785398 cos: 0.707107 sin : 0.707107
local : rad : 0.785398, cos : 0.707812, ssin : 0.707813

degree 60
math : rad : 1.047198 cos: 0.500000 sin : 0.866025
local : rad : 1.047198, cos : 0.500000 sin : 0.866667

Diff Detail

rEFL core/efl
Automatic diff as part of commit; lint not applicable.
Automatic diff as part of commit; unit tests not applicable.
jsuya created this revision.Oct 24 2019, 4:37 AM
jsuya requested review of this revision.Oct 24 2019, 4:37 AM
cedric requested changes to this revision.Oct 24 2019, 2:37 PM

I remember vaguely why this is done and it is for speed purpose. The problem seems to happen with large number, is that a valid problem for this API to solve in EFL context? EFL is mostly targeting user interface. Could you point out a scenario where this is a problem?

This revision now requires changes to proceed.Oct 24 2019, 2:37 PM
jsuya added a comment.Oct 24 2019, 6:56 PM

HI @cedric

This can happen when parsing SVG files.
Here is an SVG example that draws X.
(SVG spec does not constrain the range of values.)
white X and red X is same. but white_x is wrong displayed in efl.
This is because the values of cos(and sin) are different in rotate(45).
white x causes a lot of unnecessary computation. But I do not think this is wrong.
Because organization of an SVG file can vary depending on the tool creates SVG file.

<?xml version="1.0" encoding="utf-8"?>
<!-- Generator: Adobe Illustrator 22.1.0, SVG Export Plug-In . SVG Version: 6.00 Build 0)  -->
<svg version="1.1" id="Layer_1" xmlns="" xmlns:xlink="" x="0px" y="0px"viewBox="0 0 32 32" enable-background="new 0 0 32 32" xml:space="preserve">
<rect y="0" fill="none" width="32" height="32"/>

<g id="white_X" transform="translate(1800.284271, 28.284271) rotate(45.000000) translate(-1800.284271, -28.284271) translate(1780.284271, 8.284271)">
   <path fill="none" stroke="#FFFFFF" stroke-width="2" stroke-linecap="round" stroke-linejoin="round" enable-background="new" d="M-1250.4,1273v-8V1273h-8H-1250.4z M-1250.4,1273v8V1273h8H-1250.4z"/>

<g id="red_X" transform="translate(16 16) rotate(45) translate(-16 -16)">
   <path d=" M16,8v8V16h8H16z M16,24v-8V16h-8H16z" enable-background="new" stroke-linejoin="round" stroke-linecap="round" stroke-width="2" stroke="#FF0000" fill="none"/>

+)This is a matrix calculation that calculates the coordinates of white_x's first path position.

jsuya added a comment.Oct 28 2019, 5:53 PM

Hi @cedric please review this patch

So we do have a precision problem and we have to choose between speed and accuracy basically :-( This is not great. I will look more closely on this patch Wednesday.

btw cosf returns a float, not a double

Good point, also use of sincos would be better in this case.

maybe can be of some help

In term of performance we can expect to loose around 200 cycles waiting for the sincos to finish (On modern system, worse on older). This is to be compared to the fmod+_cos/_sin code which can slightly be executed in parallel, which I would guess put it under 50 cycles. For reference on the performance issue: . Overall, I guess we do not have much time to get this working and most of the hardware we have are not too bad with sincos. Ideally, later we would want someone to look at this and improve accuracy.

Also can you write a test that would break before and after your change for eina and add some explanation of why that #if 1 is there.

jsuya updated this revision to Diff 26762.Nov 7 2019, 10:33 PM

@cedric I added a comment. And modified the eina_matrix3_rotate test. please check again.

cedric accepted this revision.Nov 15 2019, 11:51 AM
This revision is now accepted and ready to land.Nov 15 2019, 11:51 AM
This revision was automatically updated to reflect the committed changes.