-
Notifications
You must be signed in to change notification settings - Fork 10
Accomodate new pan event changes #56
base: master
Are you sure you want to change the base?
Conversation
It seem it miss some header |
Yep, this requires revery-ui/esy-sdl2#11 to build |
@@ -471,7 +492,7 @@ module Event = { | |||
| WindowClosed(windowEvent) | |||
| WindowTakeFocus(windowEvent) | |||
| WindowHitTest(windowEvent) | |||
| MousePan(mousePan) | |||
| Pan(panEvent) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is mousePan still used ?
src/sdl2_wrapper.cpp
Outdated
@@ -717,17 +717,37 @@ CAMLprim value Val_SDL_Event(SDL_Event *event) { | |||
case SDL_PANEVENT: | |||
v = caml_alloc(1, 24); | |||
|
|||
vInner = caml_alloc(9, 0); | |||
vInner = caml_alloc(5 * 8, 0); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
5 * 8 ???? why.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
me being dumb, was adjusting number of fields and forgot to change to constant
thanks for the catch
src/sdl2_wrapper.cpp
Outdated
|
||
Store_field(vInner, 2, Val_int(event->pan.source)); | ||
int axis; | ||
if( event->pan.axis == SDL_PAN_AXIS_VERTICAL ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would suggest: switch in case we add diagonal pan. And we could get exhaustivity checking with -Wswitch-enum
flag.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thankfully I think a diagonal pan is always sum of vertical and horizontal, though agree a switch is more semantically clear here
src/sdl2_wrapper.cpp
Outdated
|
||
Store_field(vInner, 4, panElement); | ||
} else { | ||
if( event->pan.pantype == SDL_PANEVENTTYPE_INTERRUPT ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here again a switch is better: reduce nesting. Don't omit break;
src/sdl2_wrapper.cpp
Outdated
@@ -717,17 +717,39 @@ CAMLprim value Val_SDL_Event(SDL_Event *event) { | |||
case SDL_PANEVENT: | |||
v = caml_alloc(1, 24); | |||
|
|||
vInner = caml_alloc(9, 0); | |||
vInner = caml_alloc(40, 0); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
5 is enough ? panEvent
has 5 field: so 5 values.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🤦 yep, forgot it isn't malloc lol
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed
Adds fixes to accommodate changes from revery-ui/esy-sdl2#11