Bug 373101 - Incorrect reflection for cylinder animation (it uses cube reflection causing it to separate at edge)
Summary: Incorrect reflection for cylinder animation (it uses cube reflection causing ...
Status: RESOLVED FIXED
Alias: None
Product: kwin
Classification: Plasma
Component: effects-various (show other bugs)
Version: unspecified
Platform: Other Linux
: VLO minor
Target Milestone: ---
Assignee: KWin default assignee
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-11-30 07:34 UTC by David Rankin
Modified: 2018-06-01 15:35 UTC (History)
2 users (show)

See Also:
Latest Commit:
Version Fixed In:


Attachments
cube reflection looking straight-on at the desktop (1.03 MB, image/png)
2016-11-30 07:34 UTC, David Rankin
Details
cube reflection between desktops (1.06 MB, image/png)
2016-11-30 07:35 UTC, David Rankin
Details
cylinder rotation between desktops (reflection detached) (1.08 MB, image/png)
2016-11-30 07:37 UTC, David Rankin
Details
cylinder reflection looking straight-on desktop (1.11 MB, image/png)
2016-11-30 07:38 UTC, David Rankin
Details

Note You need to log in before you can comment on or make changes to this bug.
Description David Rankin 2016-11-30 07:34:27 UTC
Created attachment 102538 [details]
cube reflection looking straight-on at the desktop

If this should go with some other component, feel free to change the selected component, there isn't a desktop effects choice.

When using the cylinder animation (with zoom set at roughly 1/4), the reflection bounces up and down below the cylinder on rotation. Meaning, with reflection enabled, when you invoke the cylinder animation (e.g. win+F11 key) and tilt the cylinder toward, or away, from you about 15-20 degrees, you find the reflection properly bound to the lower edge of the desktop. Now, if you rotate the cylinder 45 degrees left/right so you are looking at the divider between desktop 1/2 (or 4/1) you see the reflection is now detached from the cylinder and it has moved down (and sometimes off the screen) depending on the amount you have the cylinder tilted toward or away from you.

If you rotate the cylinder (make it spin slowly), the reflection hops up-and-down at the bottom of the screen.

Why? The 'cube' reflection code is being used for the 'cylinder' rotation. That's not right -- they are separate pieces of code. Why? with the 'cylinder', the reflection distance from the cube does NOT change with rotation angle (you are rotating a circle, (the radius never changes), but with the 'cube' it does (e.g. the perpendicular distance from the center to the front edge is shorter than the distance from the center of the cube to the corner between virtual desktops)

Now go perform the same test with the 'cube' animation. The reflection moves up and down to stay attached to the cube rotation as it should, both when viewing a desktop straight-on (see: cube_reflection_desktop.png) and between desktops (see: cube_reflection_corner.png)

Now do it with the cylinder. The reflection is attached when viewing the desktop straight-on (see: cylinder_reflection_desktop.png), but detaches from the cylinder between desktops (see: cylinder_reflection_corner.png) The reflection should remain attached at all points during the rotation, because the distance from the center of the cylinder to the edge never changes.

It looks like somebody simply forgot to implement the cylinder rotation code from compiz (it's actually much simpler than the cube model), or it may be there, but all rotations are using the cube code for rotation (I haven't tested the sphere, it has more serious issues with the skydome deformation bug), but I suspect it is using the cube rotation code as well for reflection control.
Comment 1 David Rankin 2016-11-30 07:35:26 UTC
Created attachment 102539 [details]
cube reflection between desktops

showing proper reflection attachment at the corner of desktops with cube rotation
Comment 2 David Rankin 2016-11-30 07:37:00 UTC
Created attachment 102540 [details]
cylinder rotation between desktops (reflection detached)

Showing the reflection detached from cylinder rotation between desktops (moving down away from the cylinder because it is using the cube rotation code)
Comment 3 David Rankin 2016-11-30 07:38:24 UTC
Created attachment 102541 [details]
cylinder reflection looking straight-on desktop

showing reflection with cylinder looking straight-on the desktop (as it should be, it is attached at the bottom desktop edge)
Comment 4 Martin Flöser 2016-11-30 10:50:20 UTC
Please provide output of:
qdbus org.kde.KWin /KWin supportInformation

In general: please forget anything about Compiz. This is not compiz, we did not take any code from Compiz, thus we cannot have "forgotten" anything from Compiz. By bringing up Compiz in your bug reports you make our work way more complicated.
Comment 5 Igor Poboiko 2018-06-01 15:35:24 UTC
Git commit 6408e0ba6045d03b8872eab71060ac8d6f13ee9f by Igor Poboiko.
Committed on 01/06/2018 at 15:32.
Pushed by poboiko into branch 'master'.

[effects/cube] Fix animation and reflection issues

The main problem was that inside the effect there was manualVerticalAngle,
which did not represent the actual rotation angle of the cube during the
animation, but used to calculate the position of the reflection. The actual
angle was calculated on-the-fly and was not exposed outside.

Brief description of what the code does:

- variables currentAngle and verticalCurrentAngle now always represent the
current position of the cube. They are updated when one uses the mouse and
inside the rotateCube() method, which is called in prePaintScreen().
- two queues, animations (used for Start / Stop / Left / Right) and
verticalAnimations (used for Up / Down) are used for scheduling the animations
if i.e. user presses several keys in a row. The code checks whether the last
animation has finished (and thus we need to start a new one) inside
prePaintScreen() and postPaintScreen()
 - when the animation starts, code saves the starting position of the cube
inside startAngle, startFrontDesktop and verticalStartAngle variables, which
are used to calculate the actual cube position during the animation later.
This is done by startAnimation() and startVerticalAnimation(), which also
calculates the QTimeLine curves needed for animation
Related: bug 213599

Differential Revision: https://phabricator.kde.org/D9860

M  +256  -441  effects/cube/cube.cpp
M  +26   -19   effects/cube/cube.h

https://commits.kde.org/kwin/6408e0ba6045d03b8872eab71060ac8d6f13ee9f