Re: [Clusterviz-devel] panning proposal

[Jonas M. <na...@us...>]

Hey,

(In the following I edited your mail a little to enhance readability, as it 
did not seem to conform to the 80 character per line standard)...

On Tuesday 17 January 2006 23:15, K.W...@gm... wrote:
> Hello,
> a few minutes ago I posted two files for the panning feature. Since I made
> several changes (which I sketch below), I did not post diffs but the whole
> files. Please tell me if there is a better way to post. 

Well, now that you got an account for the SVN-server, make use of it! As soon 
as your changes do not contain any evident errors and your code behaves 
seemingly correct - commit it!
This is way better than having us check every line of your coding. Also it 
easens the way we can verify. And thirdly: as it is a revision control system 
we could easily go back to a prior state should things turn out bad. So don't 
worry.
But please try and follow some atomicity principle in the commits.

I took a look at your code and it looks generally ok! The panning works 
basically and we could leave as it is. Though I imagined a slightly different 
behavior in the beginning:
Now, when you pan, the point about which we rotate is not changed. I thought 
that the panning would also influence the point around which NEW rotations 
happen. Notice that only the new rotations would be affected. Thus it would 
probably not be possible anymore to separate the transform and center matrix 
anymore as we have it now (or the transform matrix would have to be modified 
while pannig - and it is not clear to me how this would boil down 
mathematically). My way would always perform the new rotations about the 
center of the view and therefore you could have the scene rotate about 
different points (than the mean of data) which might be desirable - or not. I 
would say this is open for discussion.

> In this proposal, I tried to keep the things simple. There is even no pan
> matrix kept, but rather the center member variable is put back in use.

This seems to be a possible way to do it for now.

> Furthermore, I moved the lastx/y member variables to the clusterdisplay
> class. They are passed as parameters to gltbZoom and gltbPan. I think this
> is a good idea since lastx/y stored a mouse state, and the clusterdisplay
> does also store some mouse state (button and state) -- so why not store it
> in one place? I also removed gltbMouse from the trackball class for the same
> reason.

We should make sure the code for user interaction is handled in one place or 
at least consistently. Let's rethink the locations.

> And there is a second reason: 
> I suggest removing all glut code from the trackball class, since maybe
> someday glut might not suffice anymore and then it would be nice to reuse
> the trackball class.

For me this reason seems minor. Do you have any specific other library in mind 
that would replace GLUT? Perhaps OpenGLContext or how it is called? This 
library semt to have some nice properties but it also was somewhat bulky and 
obstrusive.

> At last, I commented out for clarity some unused code in clusterdisplay.py
> (but all I changed were the mouse and motion methods.) And I ran a
> ":%s/;$//g" in vim on the trackball.py, for aesthetical reasons ;-)

That is principally ok - though it would have been preferable to do that in a 
different proposal / commit. Then it would have been clearer which changes 
were relevant and which were purely aesthetic.

> The scale parameter in trackball.gltbPan needs to be set more carefully,
> but this requires good knowledge of gluPerspective. If you like the overall
> proposal, I will look into it.

Go ahead! I placed the question of the scale parameter into the TODO file.

> I also tried the 4-D projection feature and it seems work pretty well! It
> raises if I choose dim 4 on a 3-dimensional data set, though. 

I will look into this.

> I also like 
> the script which generates the
> data. Wouldn't it be nice to use it directly rather than by way of a
> intermediary data file?

If you have propositions on how to do that..


I should not forget to say, that after checking your code (and slightly 
editing it: there was another variable in clusterdisplay that is now 
superfluous) I commited it. But please feel free to commit yourself in the 
future.

Regards, Jonas.