[PATCH] GUI: Potmeter button corrections
Hi Ingo, while coding the rpotmeter, I came across the following: 1) When rendering the [hv]potmeter, the button width/height is regarded, so that the button doesn't exceed the potmeter edges. However, the counterpart in the mouse handling doesn't do the exact inverse calculation. The button is drifting under the mouse pointer from its one edge to its other when moving it from one end of the potmeter to the other. Only in the middle of the pot, the button is centered under the mouse. The attached 'potmeter_button_adjust.patch' corrects the mouse calculations to keep the button always centered under the mouse (like one might expect). The coming rpotmeter button will have the same behaviour (inherent by design). 2) The WIN32 GUI checks that the rendered potmeter button doesn't exceed the bounds. This seems not necessary as the item value is already limited. The GTK GUI doesn't do this check neither. The attached 'potmeter_button_cleanup.patch' removes those checks, simplifying the code. Hans-Dieter
Hans-Dieter Kosch wrote on Mon, 24 Mar 2014 01:20:57 +0100:
while coding the rpotmeter, I came across the following:
2) The WIN32 GUI checks that the rendered potmeter button doesn't exceed the bounds. This seems not necessary as the item value is already limited. The GTK GUI doesn't do this check neither.
That's right.
The attached 'potmeter_button_cleanup.patch' removes those checks, simplifying the code.
Applied. Ingo
Hans-Dieter Kosch wrote on Mon, 24 Mar 2014 01:20:57 +0100:
while coding the rpotmeter, I came across the following:
1)
When rendering the [hv]potmeter, the button width/height is regarded, so that the button doesn't exceed the potmeter edges. However, the counterpart in the mouse handling doesn't do the exact inverse calculation. The button is drifting under the mouse pointer from its one edge to its other when moving it from one end of the potmeter to the other. Only in the middle of the pot, the button is centered under the mouse.
The attached 'potmeter_button_adjust.patch' corrects the mouse calculations to keep the button always centered under the mouse (like one might expect).
Index: gui/ui/main.c =================================================================== +++ gui/ui/main.c (working copy) @@ -148,10 +148,10 @@ switch( itemtype ) { case itHPotmeter: - value=100.0 * ( X - item->x ) / item->width; + value=100.0 * ( X - item->x - item->pwidth / 2.0 ) / ( item->width - item->pwidth ); break; case itVPotmeter: - value=100.0 - 100.0 * ( Y - item->y ) / item->height; + value=100.0 - 100.0 * ( Y - item->y - item->pheight / 2.0 ) / ( item->height - item->pheight ); break; } uiEvent( item->message,value );
This will go beyond the boundaries, resulting in an uiEvent() call with an illegal value. I'm not against a "centered" potmeter button, but I don't think that this is the right place to achieve this. The job of the mouse handler is to return the exact mouse position, not to "prettify" it. It's the job of the renderer to interpret the item properties and to produce a pleasant result. Seems I really have to commence revising render.c now... Ingo
Ingo Brückl wrote:
Hans-Dieter Kosch wrote on Mon, 24 Mar 2014 01:20:57 +0100:
The attached 'potmeter_button_adjust.patch' corrects the mouse calculations to keep the button always centered under the mouse (like one might expect).
Index: gui/ui/main.c =================================================================== +++ gui/ui/main.c (working copy) @@ -148,10 +148,10 @@ switch( itemtype ) { case itHPotmeter: - value=100.0 * ( X - item->x ) / item->width; + value=100.0 * ( X - item->x - item->pwidth / 2.0 ) / ( item->width - item->pwidth ); break; case itVPotmeter: - value=100.0 - 100.0 * ( Y - item->y ) / item->height; + value=100.0 - 100.0 * ( Y - item->y - item->pheight / 2.0 ) / ( item->height - item->pheight ); break; } uiEvent( item->message,value );
This will go beyond the boundaries, resulting in an uiEvent() call with an illegal value.
You're right. I missed that. There's no value range check like in 'case wsMoveMouse:' (nevertheless, I noticed exactly the expected behaviour, though (?) ).
I'm not against a "centered" potmeter button, but I don't think that this is the right place to achieve this.
The job of the mouse handler is to return the exact mouse position, not to "prettify" it. It's the job of the renderer to interpret the item properties and to produce a pleasant result.
Agreed. The issue is currently that we have an indirect feedback from the mouse position to the position of the related displayed object. The mouse position yields an item value, and then in turn, the item value yields an object position. If the forward and reverse calculations differ, arbitrarily wierd displays can result.
Seems I really have to commence revising render.c now...
I'm looking forward... PS: The rpotmeter is working now at least for the GTK GUI (I have still nothing to test Win32). But before posting, I like to review and see if changes result from the current discussions. Hans-Dieter
Hans-Dieter Kosch wrote on Wed, 26 Mar 2014 01:05:27 +0100:
Ingo Brückl wrote:
This will go beyond the boundaries, resulting in an uiEvent() call with an illegal value.
You're right. I missed that. There's no value range check like in 'case wsMoveMouse:'
In this case value will always remain within the limits (which isn't true for the mouse move).
(nevertheless, I noticed exactly the expected behaviour, though (?) ).
btnModify() (called prior to rendering) finally restricts value.
The mouse position yields an item value, and then in turn, the item value yields an object position. If the forward and reverse calculations differ, arbitrarily wierd displays can result.
Currently we actually have: mouse position -> value (which is decisive) -> item representation (by rendering). I'm aware that there may be precision problems when using a potmeter to adjust something, but so far this didn't cause major troubles.
PS: The rpotmeter is working now at least for the GTK GUI (I have still nothing to test Win32).
Don't spend too much time dealing with Win32. I can do the tests, and we can even implement it for Win32 later (though I'd like to implement at one go). Ingo
Ingo Brückl wrote:
Hans-Dieter Kosch wrote on Wed, 26 Mar 2014 01:05:27 +0100:
Ingo Brückl wrote:
This will go beyond the boundaries, resulting in an uiEvent() call with an illegal value.
You're right. I missed that. There's no value range check like in 'case wsMoveMouse:'
In this case value will always remain within the limits (which isn't true for the mouse move).
But uiEvent() is really called with an out of range value.
(nevertheless, I noticed exactly the expected behaviour, though (?) ).
btnModify() (called prior to rendering) finally restricts value.
So, possibly restrict the value as done in wsMoveMouse before calling uiEvent() (to be formally correct) and implement this patch or a variant as at least an intermediate solution?
The mouse position yields an item value, and then in turn, the item value yields an object position. If the forward and reverse calculations differ, arbitrarily wierd displays can result.
Currently we actually have: mouse position -> value (which is decisive) -> item representation (by rendering).
At this point coming into my mind: The button position is also updated by 'external' item value changes, e.g. movie position while playing, or volume/balance changed by a concurrently running audio mixer. This works perfectly so far. So, the item value appears really to be the correct reference for rendering. And thus, the translation 'mouse position -> value' should match the translation 'value -> rendering position'. The current problem seems to me, that the related calculations are scattered loosely across the files; maybe this could be made consistent by defining appropriate macros in a common header file.
I'm aware that there may be precision problems when using a potmeter to adjust something, but so far this didn't cause major troubles.
Yes, of course, I didn't mention, as it appeared obvious to me. And precision is even more reduced with my patch. But that's physical constraint anyway, a high res screen gives finer control than a low res one.
PS: The rpotmeter is working now at least for the GTK GUI (I have still nothing to test Win32).
Don't spend too much time dealing with Win32. I can do the tests, and we can even implement it for Win32 later (though I'd like to implement at one go).
OK. I'll post latest at next weekend. It's my habit to let settle and review, and push out related things all together. You can of course at your discretion hold off committing until Win32 is worked out. Hans-Dieter
Hans-Dieter Kosch wrote on Thu, 27 Mar 2014 02:12:25 +0100:
The current problem seems to me, that the related calculations are scattered loosely across the files; maybe this could be made consistent by defining appropriate macros in a common header file.
This is mainly because large parts of the source files haven't yet been properly revised. Currently the GUI code is a mixure of different coding styles, checked code that (hopefully) is where it should be and where you would also expect it to be, and often redundant code at different and not always appropriate places. There are two main problems with improving this: A constant lack of time and the attempt to not break things while sometimes rebuilding it to a certain extend. Ingo
Hans-Dieter Kosch wrote on Wed, 26 Mar 2014 01:05:27 +0100:
Ingo Brückl wrote:
Seems I really have to commence revising render.c now...
I'm looking forward...
Done.
The job of the mouse handler is to return the exact mouse position, not to "prettify" it. It's the job of the renderer to interpret the item properties and to produce a pleasant result.
Agreed.
I was just going to commit this patch when I realized that this will cause a very unpleasant issue. With a centered button all values from 0 to buttonsize/2 percent and 100-buttonsize/2 to 100 percent will end up in the same drawn button position, or - to put it another way - dragging the button to the edges will result in value changes but no button movement (for the width of buttonsize/2). So, in weighing up the pros and cons of a "drifting" mouse pointer with a definite (and different) position for all possible values and a centered one that allows value changes without changing its position, I'm clearly going for the "drifting" one. Ingo
Ingo Brückl wrote:
I was just going to commit this patch when I realized that this will cause a very unpleasant issue.
With a centered button all values from 0 to buttonsize/2 percent and 100-buttonsize/2 to 100 percent will end up in the same drawn button position, or - to put it another way - dragging the button to the edges will result in value changes but no button movement (for the width of buttonsize/2).
Yes, I see. It's due to the min/max constraints in rendering. The tie is that mouse handling and rendering are dependent on each other to obtain a correct visual feedback. Both have to regard the item dimensions and have to do matching forward/reverse calculations. Both do regard the potmeter size, but only rendering regards the potmeter's button size, while mouse does not. This is what my suggested patch aims to fix. This could be coded by a common include-file, binding those dependencies clearly together. Hans-Dieter
Hans-Dieter Kosch wrote on Sun, 30 Mar 2014 01:51:46 +0100:
Ingo Brückl wrote:
I was just going to commit this patch when I realized that this will cause a very unpleasant issue.
With a centered button all values from 0 to buttonsize/2 percent and 100-buttonsize/2 to 100 percent will end up in the same drawn button position, or - to put it another way - dragging the button to the edges will result in value changes but no button movement (for the width of buttonsize/2).
Yes, I see. It's due to the min/max constraints in rendering.
The tie is that mouse handling and rendering are dependent on each other to obtain a correct visual feedback. Both have to regard the item dimensions and have to do matching forward/reverse calculations. Both do regard the potmeter size, but only rendering regards the potmeter's button size, while mouse does not. This is what my suggested patch aims to fix.
A correct fix would be to make the middle of the button the representative for the item's value, but that would result in a button that would stick out at the edges half a button size. While this is exactly what a real poti does, this unfortunately never seemed to be the design plan for the GUI and thus no skin is prepared to handle it. So yes, agreed, there is no correct visual feedback regarding a real life poti, but I never had the feeling (as a GUI user) that the potmeter button behaves strangely or incorrect. Ingo
Ingo Brückl wrote:
A correct fix would be to make the middle of the button the representative for the item's value, but that would result in a button that would stick out at the edges half a button size.
Middle of the button, right, that's what my patch does in mouse capturing; the rendering is already correct. And this way, the button will never exceed the edges. I understand your aim for abstraction, but both capturing and rendering are bound to the same screen positions, and the related calculations must match; I see no way around.
While this is exactly what a real poti does, this unfortunately never seemed to be the design plan for the GUI and thus no skin is prepared to handle it.
What do you mean by design plan? A skin definition gives the layout, but placing variable elements inside the skin like a potmeter button is finally handled by the code, of course regarding the constraints given by the skin definition. Hans-Dieter
Hans-Dieter Kosch wrote on Tue, 01 Apr 2014 04:04:04 +0200:
Ingo Brückl wrote:
A correct fix would be to make the middle of the button the representative for the item's value, but that would result in a button that would stick out at the edges half a button size.
Middle of the button, right, that's what my patch does in mouse capturing; the rendering is already correct. And this way, the button will never exceed the edges.
Yes, but your patch shortens by a button size the part of the potmeter slide area that returns different values, or - to put it another way - the areas half a button size at the edges return the same values, i.e. 0% is everywhere between the left edge and half a button size at the right. Compared with the current behaviour, *that* would be confusing. Where would I have to click for a little bit over 0%? Certainly not a few pixels right to the left edge. This is still 0%.
I understand your aim for abstraction, but both capturing and rendering are bound to the same screen positions, and the related calculations must match; I see no way around.
Maybe I'm getting your point when I'm looking over the rpotmeter patch, but for the current potmeters I really don't (and never did) see a problem (which, of course, could be because I'm used to their behaviour).
While this is exactly what a real poti does, this unfortunately never seemed to be the design plan for the GUI and thus no skin is prepared to handle it.
What do you mean by design plan?
I meant that the people that had set up the rules for skins didn't plan to have potmeter buttons that stick out at the edges, hence all skin designs rely on it and a button sticking out may overlap or come too close to an other skin item then. Ingo
Hans-Dieter Kosch wrote on Mon, 24 Mar 2014 01:20:57 +0100:
The attached 'potmeter_button_adjust.patch' corrects the mouse calculations to keep the button always centered under the mouse (like one might expect).
Index: gui/win32/gui.c =================================================================== +++ gui/win32/gui.c (working copy) @@ -927,20 +927,22 @@
The Win32 GUI seemingly wants to "stick" the mouse pointer to the potmeter button so that the mouse points to the same position inside the button before and after the move (the Win32 GUI can only move the button, not click inside the potmeter area). Ingo
Ingo Brückl wrote:
Hans-Dieter Kosch wrote on Mon, 24 Mar 2014 01:20:57 +0100:
The attached 'potmeter_button_adjust.patch' corrects the mouse calculations to keep the button always centered under the mouse (like one might expect).
Index: gui/win32/gui.c =================================================================== +++ gui/win32/gui.c (working copy) @@ -927,20 +927,22 @@
The Win32 GUI seemingly wants to "stick" the mouse pointer to the potmeter button so that the mouse points to the same position inside the button before and after the move (the Win32 GUI can only move the button, not click inside the potmeter area).
The patch was plain theory of mine from formal comparisons. I have no installation to study win32. I must look deeper, there's obviously impact on the rpotmeter implementation. Hans-Dieter
participants (2)
-
Hans-Dieter Kosch -
Ingo Brückl