suspicious use of modulo operator about qtubuntu-camera

[peat-psuwit]

Well, % 360 is used to catch the case when orientation is zero. In that case, if modulo isn't used, we'll return 360 - 0 = 360, which doesn't make sense.

As far as I know, both ranges of Android's and Qt's value are {0, 90, 180, 270}. That is, only these 4 values are valid. The orientation is intended to tell how the camera is mounted inside the device, and I think no manufacturer is crazy enough to put the sensor diagonally.

So, I think it's OK to leave it as is. However, if someone else finds any information that suggests otherwise, feel free to raise that.

[lenoleno]

Sounds reasonable. I haven't been aware that here angle can only be a multiple of 90 degrees.

One other thing: As long as orientation is known to be from the set {0, 90, 180, 270} catching the zero orientation case by a conditional should be faster than employing the modulo operator. (Last time I did ARM assembler was two decades ago [on ARM3], but I hope things didn't change too much in this respect.)

Since ARM can do conditional execution (no need to branch and empty a pipeline), a compiler should translate

if (orientation != 0) {
    orientation = 360 - orientation
}

into something along the lines

CMP R0, #0
RSBNE R0, R0, #360

which consumes two cycles. (Assuming #360 is a valid immediate constant in ARM assembler, which I think it is. Valid constants are/were of the form x << n where x is an 8 bit value and n is an even shift. 360 = 90 << 2.) And perhaps even the zero test could be optimized away by a compiler, leaving us with just one cycle for the whole transformation. Doing an integer division (modulo) should be much slower. (Well, ARM3 didn't even have an integer division unit back then, which is why one got used to employ all possible tricks to avoid them. :-)

Since this is a camera app, we should aim for snappiness, wherever possible, no? Otherwise "all those moments" are at risk to be "lost in time like tears in rain." (RIP, Rutger Hauer!)

[peat-psuwit]

Hello,

Thank you for contributing to UBports. As part of project renaming and the effort to port Ubuntu Touch stack to Ubuntu 20.04, we're incrementally migrating repositories to GitLab.

Your issue is now migrated to:
https://gitlab.com/ubports/core/qtubuntu-camera/-/issues/7

Sorry for your inconvenience.