shoes / shoes3

a tiny graphical app kit for ruby
http://walkabout.mvmanila.com
Other
181 stars 19 forks source link

Shoes.app click event coordinates are reset on elements #329

Closed IanTrudel closed 6 years ago

IanTrudel commented 7 years ago

The coordinates on a Shoes.app main click event are offset back to zero over elements. Clicking on each corners and then on the button as seen on the screenshot. The coordinates are (42, 18) even though the button is in the middle of the window.

Shoes.app do
   stack do
      @p = para
   end

   stack(left: self.width / 2, top: self.height / 2) do
      button("hello")
   end

   click do |btn, left, top|
      @p.text += "click: #{btn}, #{left}, #{top}\n"
   end
end

image

IanTrudel commented 7 years ago

Noteworthy to mention that Shoes.app click events are ignored when the button state is disabled.

IanTrudel commented 7 years ago

I have tracked down the problem to GTK. Every widget (click button, motion, wheel) event contains relative coordinates. It may be a desirable behaviour for GTK but is not for Shoes considering the click event is attached to Shoes.app rather than a button itself. @ccoupe I would like your feedback before making any permanent changes. What is the behaviour on macOS ?

Changing shoes_app_gtk_button proved successful:

 static gboolean shoes_app_gtk_button(GtkWidget *widget, GdkEventButton *event, gpointer data) {
     shoes_app *app = (shoes_app *)data;
     shoes_canvas *canvas;
     Data_Get_Struct(app->canvas, shoes_canvas, canvas);

     int x, y;
     GdkModifierType state;

     gdk_window_get_device_position(gtk_widget_get_window(widget), event->device, &x, &y, &state);

     if (event->type == GDK_BUTTON_PRESS) {
         shoes_app_click(app, event->button, x, y);
     } else if (event->type == GDK_BUTTON_RELEASE) {
         shoes_app_release(app, event->button, x, y);
     }
     return TRUE;
 }
IanTrudel commented 7 years ago

Testing motion works as intended, no relative coordinates.

ccoupe commented 7 years ago

What are the backwards compatibility implications? What does/can OSX do (as you noted)? Premature push.

IanTrudel commented 7 years ago

It's absolutely undesirable behaviour and I suspect it went unnoticed for as long as it exists. It became clear when motion had correct coordinates.

You should consider that in Shoes normal button usage would deal with clicks using its own block rather than a global click event.

I only noticed this issue when I implemented a UI builder. Very unlikely to cause backward compatibility issues.

ccoupe commented 7 years ago

Cocoa.m: line 122 is widget agnostic. Odds are this is the reason Gtk behaves the way it does. Should the proposed new behavior apply to all Gtk widgets?

The premature commit will cause much trouble for osx code and does not belong on the master branch at this time. You can retract the commit, create a branch and remote tracking for it or I can reject the commit in which case you'll lose your code when you next pull. That is not optimal for either of us.

@BackOrder do you have a fork or a clone of Shoes3? I need to fix these git issues so this won't happen again - your 'push' should create a pull request and not go straight to master w/o review. I hope you understand.

[updated]

IanTrudel commented 7 years ago

Cocoa.m: line 122 is widget agnostic. Odds are this is the reason Gtk behaves the way it does. Should the proposed new behavior apply to all Gtk widgets?

Yes. The good news is that the commit does fix other widgets. (see test code below)

RE: OSX code

My understanding is that OSX is not afflicted by the problem. Most windowing API just don't do this. This behaviour is very specific to GTK.

RE: Shoes 3 fork/clone

I do. It does make sense when there are bigger code at stake, such as refactoring C code or your current widget thing (which should be noted as a branch of Shoes3 rather than a fork). However, it's a lot more work on my end for small commits. It's different working from a branch versus a fork. Syncing a fork with its parent, conflicts, merge, create pull request on top of normal pull/push isn't all that fun.

Now if you insist, I will oblige. I just think small things should remain small. We're talking about a five-liners commit here.

Sample code tested with/without the fix:

Shoes.app do
   stack do
      @p = para
   end

   stack(left: self.width / 4, top: 10) do
      button("hello")
      flow { check; para "check?" }
      edit_box
      edit_line
      list_box :items => ["one", "two", "three"]
      progress
      stack do
          radio; para strong("one")
          radio; para strong("two")
      end
      slider
      spinner start: true
      switch
   end

   click do |btn, left, top|
      @p.text = "click: #{btn}, #{left}, #{top}\n"
   end
end
ccoupe commented 7 years ago

Re: the sample (now bugs/bug329.rb) - W/o the patch, linux, windows and osx behave identically - clicking in the widgets does not produce an click display except except progress widget (all platforms) and that seems to be in window co-ordinates. OK - the widget swallows the clicks (except progress). The very first example does the same. Clicking in the middle of the button does not trigger the click block. That is not what you reported way up top. Clearly, you saw something, but I can't recreate it. Changing the co-ords sent to shoes_canvas_send_click doesn't seem to be a good idea.

ccoupe commented 7 years ago

freebsd does not exhibit the problems reported. Just for fun: editbox does trigger the app click block when it shouldn't. Smells like Gtk3 version/implementation behavior. Also spinner doesn't show on my Mate default theme. With the patch, linux now trigger the app click block for spinner?

IanTrudel commented 7 years ago

Edit widgets swallow right clicks but will respond to left clicks. Other widgets will respond to right click but swallow left clicks.

There might be a case to be made that global click event is inconsistent. We might consider that it should always be triggered.

freebsd does not exhibit the problems reported.

It does here on all Shoes (custom build and yours) and on Windows.

Also spinner doesn't show on my Mate default theme. With the patch, linux now trigger the app click block for spinner?

Maybe some icons are missing or MATE assumes the style of stopped spinner is display nothing. It wasn't displayed here until I set it to start true.

ccoupe commented 7 years ago

Ahh - other buttons! Now I understand.

There might be a case to be made that global click event is inconsistent. We might consider that it should always be triggered.

Inconsistent indeed. I think it would be good if buttons other that '1' did trigger the app's click block and not activate the widget. (left click is button 1 for me - the one I use to push a button), We are in undefined territtory. My mac and linux boxes report differed button numbers for right and center (2 vs 3, 3 vs 2).

IanTrudel commented 7 years ago

In theory, it would be best (and easiest) to passthru any clicks to the global click event handler. It means the global click event would be agnostic of all widgets. Then the Shoes programmer will have to code their app accordingly. A click on a button would trigger both the button click handler and the global click event handler.

There is an argument to be made to passthru everything except left click (1) for sake of simplicity. Slightly easier for the Shoes programmer to code. Now a left click on a button would not trigger the global click event handler.

Take note the same problem may apply to release.

We are in undefined territtory.

It is difficult to know what were the intentions of the global click event in the first place. It is not stipulated anywhere in the manual that widgets would interfere in any way, so it would make it easy to assume it is supposed to passthru all clicks.

I am leaning towards the first approach because this is the most powerful. Which approach are you leaning towards?

ccoupe commented 7 years ago

it is difficult to know what were the intentions of the global click event in the first place.

That's a Softball pitch :-). Simple graphics games - tankspank and the like and a few samples/simple that follow the mouse.

Which approach are you leaning towards?

I want to keep the beginner simplicity/approachability of Shoes. So for widgets, button 1 does exactly what it does now. - trigger widgets action and be able to shoot the cannon in tankspank..

I see the value of having mouse (1) events pass through app.click as well - I'm not sure gtk or cocoa can do that without additional code in each the native widgets to forward their 'hit' to app.click() or perhaps all could be passed to an listener instead of app.click. Let that percolate. Might be fun.

ccoupe commented 6 years ago

Closing.