bugGnome Chemistry Utils - Bugs: bug #42977, gchem3d 0.14.8 buggy molecule...

 
 

You are not allowed to post comments on this tracker with your current authentication level.

bug #42977: gchem3d 0.14.8 buggy molecule rotation

Submitted by:  Toni Andjelkovic <andjelkovic>
Submitted on:  Wed 13 Aug 2014 06:55:44 PM UTC  
 
Category: NoneSeverity: 3 - Normal
Item Group: NoneStatus: Fixed
Privacy: PublicAssigned to: Jean Bréfort <jean_brefort>
Open/Closed: Closed

(Jump to the original submission Jump to the original submission)

Mon 08 Sep 2014 08:33:46 PM UTC, comment #13:

The code comes from an old study I made about rotation arond a fixed point about twenty years ago.

I'll add comments when can remember how it wors.

I never intended to allow translation of the molecule.

Jean Bréfort <jean_brefort>
Project AdministratorIn charge of this item.
Mon 08 Sep 2014 08:01:16 PM UTC, comment #12:

I have found that the linear transformation code (in libs/gcu/matrix.cc) is a terrible mess.

To rotate a point with coordinates (x, y, z) by some angle θ along a given axis (x, y, or z), you have to multiply it with the corresponding transformation matrix:

Rx(θ) = ( 1, 0, 0,
0, cos(θ), -sin(θ),
0, sin(θ), cos(θ) )

Ry(θ) = ( cos(θ), 0, sin(θ),
0, 1, 0,
-sin(θ), 0, cos(θ) )

Rz(θ) = ( cos(θ), -sin(θ), 0,
sin(θ), cos(θ), 0,
0, 0, 1 )

I = ( 1, 0, 0,
0, 1, 0,
0, 0, 1 )

In the code, these matrices are somehow lumped together and hard-coded into two composite matrices called "euler" and "antieuler". For rotation, a multiplicative transformation is generated on the fly by multiplication of "euler" with "antieuler". I am still trying to figure out what those "euler" matrices are (Rx(ψ) x Ry(θ) x Rz(φ)?)

Having no identity operation, the system is comparable to the system of real numbers, but without the number one, so that the result of (x * 1) is undefined. In addition, the system only allows multiplication, but no addition. Therefore, you can only rotate a molecule, but you cannot not move it (which would be nice, for example by using the right mouse button).

In any case, the linear transformation code needs to be restructured like above to make it more comprehensible, extendable and less buggy. Those changes are more important than details like preference dialogs and GL shading.

There should also be a link to a source explaining the algorithm. Many trivial things in the code are commented, but the significant ones aren't. The transformation matrices above are from Banchoff/Wermer, Linear Algebra through Geometry, Springer, 1983, UTM 44.

Toni Andjelkovic <andjelkovic>
Sun 31 Aug 2014 07:59:52 AM UTC, comment #11:

Closed after 0.14.9 release.

Jean Bréfort <jean_brefort>
Project AdministratorIn charge of this item.
Sat 30 Aug 2014 09:12:39 AM UTC, comment #10:

Patch commited. Thanks.

Jean Bréfort <jean_brefort>
Project AdministratorIn charge of this item.
Fri 29 Aug 2014 07:20:46 PM UTC, comment #9:

GLView::Rotation() does not support 0,0 as arguments (divide by 0 and we get NAN), I'll fix that too, but there is no need to update the view if no change occured.

(file #31997)

Jean Bréfort <jean_brefort>
Project AdministratorIn charge of this item.
Fri 29 Aug 2014 07:01:38 PM UTC, comment #8:

OK, I have not commited anything yet. I applied locally your patch with some minor modifications, but not tested yet. I intend to commit tomorrow if time permits.

Jean Bréfort <jean_brefort>
Project AdministratorIn charge of this item.
Fri 29 Aug 2014 06:41:05 PM UTC, comment #7:

In my patch, I left out these two lines, because they looked like a workaround:

+ if ((event->x == View->m_Lastx) && (event->y == View->m_Lasty))
+ return false;

Leaving them out makes the molecule disappear when you hit the screen border while dragging the molecule.

That means that View->Rotate(0.0, 0.0) does not work as expected. In my opinion it should: a no-op rotation (0, 0) is a valid transformation that should not break the molecule view. It occurs rarely and there is no need to optimize it out. It is an identity matrix multiplication, which should happen elsewhere.

Did you commit anything related to this? If yes, please upload the patch so I can test it.

Toni Andjelkovic <andjelkovic>
Thu 21 Aug 2014 12:05:06 PM UTC, comment #6:

I see, I should replace all gdouble instances in the code, I suppose. Thanks.

Jean Bréfort <jean_brefort>
Project AdministratorIn charge of this item.
Thu 21 Aug 2014 11:16:26 AM UTC, comment #5:

Because of consistency with the signature of void GLView::Rotate (gdouble x, gdouble y) in libs/gcu/glview.cc. Also, because the x and y coordinates in GdkEventMotion are gdouble.

Toni Andjelkovic <andjelkovic>
Thu 21 Aug 2014 11:10:03 AM UTC, comment #4:

While >I'm here, why did you replace double with gdouble?

Jean Bréfort <jean_brefort>
Project AdministratorIn charge of this item.
Thu 21 Aug 2014 11:07:01 AM UTC, comment #3:

I am unable to reproduce using current debian (unstable). Anyway, I understand the logic of the patch and will apply it (or may be alightly modified version) a soon as possible.

Jean Bréfort <jean_brefort>
Project AdministratorIn charge of this item.
Thu 21 Aug 2014 10:47:36 AM UTC, comment #2:

The bug is easily reproducible on Fedora 19 and 20 with the latest packaged gtk3 versions (gtk3-3.10.9-1 on Fedora 20). It occurs accidentally when you try to rotate a molecule quickly along any axis.

Here is a detailed instruction:

Open up any molecule in the viewer (e. g. Tools menu -> Import molecule ... -> Enter "CCO", the SMILES code for ethanol -> Apply).

Rotate the molecule diagonally from the lower left corner to the upper right corner of the viewer window:

Move the mouse pointer to the lower left corner of the window. Rotate the molecule to the upper right corner of the window, but take care that the mouse pointer is moving when you left-click ("sliding left click"). Slowly moving the mouse while clicking will suffice. Finish the rotation by dragging the mouse pointer to the upper right corner of the window. Release the left mouse button.

Continue the previous rotation. Move the mouse pointer back to the lower left corner of the window. Start the rotation by moving the mouse pointer diagonally up and right, then click the left mouse button while moving (sliding left click). You'll see that the molecule gets jerked back to the start of the previous rotation.

Toni Andjelkovic <andjelkovic>
Wed 13 Aug 2014 07:56:54 PM UTC, comment #1:

I'm unable to reproduce. Does this happen with any Gtk+-3.0 version? Which one are you using?

Jean Bréfort <jean_brefort>
Project AdministratorIn charge of this item.
Wed 13 Aug 2014 06:55:44 PM UTC, original submission:

There is a bug with molecule rotation in the gchem3d viewing window:

If you click the left mouse button while moving the mouse, the rotation coordinates do not get reset properly, which results in the molecule being jerked to its old position.

This is because GTK fires the mouse motion event (with GDK_BUTTON1_MASK set) before the corresponding mouse button press event, but the code in the motion event handler assumes otherwise.

Since GTK does not provide the desired event synchronisation, you need a semaphore. The enclosed patch fixes that.

Toni Andjelkovic <andjelkovic>

 

Attached Files
file #31997:  rotation.patch added by jean_brefort (3KiB - text/x-patch - updated patch.)
file #31898:  patch-rotation.txt added by andjelkovic (3KiB - text/plain)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by andjelkovic (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can click here to add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 5 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Sun 31 Aug 2014 07:59:52 AM UTCjean_brefortOpen/ClosedOpen=>Closed
    Sat 30 Aug 2014 09:12:39 AM UTCjean_brefortStatusNone=>Fixed
      Assigned toNone=>jean_brefort
    Fri 29 Aug 2014 07:20:46 PM UTCjean_brefortAttached File-=>Added rotation.patch, #31997
    Wed 13 Aug 2014 06:55:44 PM UTCandjelkovicAttached File-=>Added patch-rotation.txt, #31898

    Back to the top


    Powered by Savane 3.1-cleanup1