Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP: First version of touch events #142

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
110 changes: 106 additions & 4 deletions blueprint/core/blueprint_View.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,33 @@ namespace blueprint
}, view);
}

juce::var makeViewEventFromTouchList(const juce::Array<blueprint::View::Touch*> &touches, const juce::Array<blueprint::View::Touch*> &targetTouches, const juce::Array<blueprint::View::Touch*> &changedTouches, const blueprint::View &view)
{
juce::var touchesVar;
for(blueprint::View::Touch* t: touches)
{
touchesVar.append(t->getVar());
}

juce::var targetTouchesVar;
for(blueprint::View::Touch* t: targetTouches)
{
targetTouchesVar.append(t->getVar());
}

juce::var changedTouchesVar;
for(blueprint::View::Touch* t: changedTouches)
{
changedTouchesVar.append(t->getVar());
}

return makeViewEventObject({
{"touches", touchesVar},
{"targetTouches", targetTouchesVar},
{"changedTouches", changedTouchesVar},
}, view);
}

}

//==============================================================================
Expand Down Expand Up @@ -225,18 +252,93 @@ namespace blueprint

void View::mouseDown (const juce::MouseEvent& e)
{
dispatchViewEvent("onMouseDown", detail::makeViewEventObject(e, *this));
switch (e.source.getType())
{
case juce::MouseInputSource::InputSourceType::mouse:
dispatchViewEvent("onMouseDown", detail::makeViewEventObject(e, *this));
break;
case juce::MouseInputSource::InputSourceType::touch: {
Touch* t = new Touch(e.source.getIndex(), e.x, e.y, e.getScreenX(), e.getScreenY(), getViewId());
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So we're using raw new here and then storing the allocated pointer in a juce::Array. To my knowledge juce::Array is non-owning. As a result there is a memory leak here as I see no matching delete anywhere for these Touch objects.

See juce::Array docs:

"Holds a resizable array of primitive or copy-by-value objects"

I would question why we need to use Touch* here. Touch is a struct and not a polymorphic base or similar so this code should probably use a value type array juce::Array<Touch> and operate with references to a "copy-by-value" type.

touches.add(t);
targetTouches.add(t);

int indexInChangedTouches = getTouchIndexInList(e.source.getIndex(), changedTouches);
if (indexInChangedTouches == -1)
changedTouches.add(t);

dispatchViewEvent("onTouchStart", detail::makeViewEventFromTouchList(touches, targetTouches, changedTouches, *this));
changedTouches.clear();
break;
}
case juce::MouseInputSource::InputSourceType::pen:
break;
default:
break;
}


}

void View::mouseUp (const juce::MouseEvent& e)
{
dispatchViewEvent("onMouseUp", detail::makeViewEventObject(e, *this));
switch (e.source.getType())
{
case juce::MouseInputSource::InputSourceType::mouse:
dispatchViewEvent("onMouseUp", detail::makeViewEventObject(e, *this));
break;
case juce::MouseInputSource::InputSourceType::touch: {
Touch* t = touches.removeAndReturn(getTouchIndexInList(e.source.getIndex(), touches));

int indexInTargetTouches = getTouchIndexInList(e.source.getIndex(), targetTouches);
if (indexInTargetTouches != -1)
targetTouches.remove(indexInTargetTouches);

int indexInChangedTouches = getTouchIndexInList(e.source.getIndex(), changedTouches);
if (indexInChangedTouches == -1)
changedTouches.add(t);

dispatchViewEvent("onTouchEnd", detail::makeViewEventFromTouchList(touches, targetTouches, changedTouches, *this));
changedTouches.clear();
break;
}
case juce::MouseInputSource::InputSourceType::pen:
break;
default:
break;
}
}

void View::mouseDrag (const juce::MouseEvent& e)
{
// TODO: mouseDrag isn't a dom event... is it?
dispatchViewEvent("onMouseDrag", detail::makeViewEventObject(e, *this));
switch (e.source.getType())
{
case juce::MouseInputSource::InputSourceType::mouse:
// TODO: mouseDrag isn't a dom event... is it?
dispatchViewEvent("onMouseDrag", detail::makeViewEventObject(e, *this));
break;
case juce::MouseInputSource::InputSourceType::touch: {
Touch* t = getTouch(e.source.getIndex(), touches);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From a style point of view some white space between lines here would be nice to assist readability.

t->setCoordinatesFromEvent(e);
auto currentTargetUnderMouse = detail::getMouseEventRelatedTarget(e, *this);
int indexInTargetTouches = getTouchIndexInList(e.source.getIndex(), targetTouches);
if (indexInTargetTouches == -1 && t->target == currentTargetUnderMouse)
targetTouches.add(t);
else if (indexInTargetTouches != -1 && t->target != currentTargetUnderMouse)
targetTouches.remove(indexInTargetTouches);

int indexInChangedTouches = getTouchIndexInList(e.source.getIndex(), changedTouches);
if (indexInChangedTouches == -1)
changedTouches.add(t);

dispatchViewEvent("onTouchMove", detail::makeViewEventFromTouchList(touches, targetTouches, changedTouches, *this));
changedTouches.clear();
break;
}
case juce::MouseInputSource::InputSourceType::pen:
break;
default:
break;
}
}

void View::mouseDoubleClick (const juce::MouseEvent& e)
Expand Down
66 changes: 66 additions & 0 deletions blueprint/core/blueprint_View.h
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,72 @@ namespace blueprint
/** Invokes, if exists, the respective view event handler. */
void dispatchViewEvent (const juce::String& eventType, const juce::var& e);

//==============================================================================
/** Represents a Touch object. */
struct Touch
{
Touch (const int identifier, const int x, const int y, const int screenX, const int screenY, const juce::var target)
: identifier(identifier), x(x), y(y), screenX(x), screenY(screenY), target(target)
{}

void setCoordinatesFromEvent(const juce::MouseEvent& e)
{
x = e.x;
y = e.y;
screenX = e.getScreenX();
screenY = e.getScreenY();
}

juce::var getVar()
{
juce::String jsonString = "{\"identifier\": " + juce::String(identifier) + ",\"x\": " + juce::String(x)+ ",\"y\": " + juce::String(y)+ ",\"screenX\": " + juce::String(screenX)+ ",\"screenY\": " + juce::String(screenY)+ ",\"target\": " + target.toString() + "}";
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems a shame to have this manual building of a JSON string here. We should be able to convert a Touch to a juce::DynamicObject instance.

return juce::JSON::parse(jsonString);
}

int identifier;
int x;
int y;
int screenX;
int screenY;
juce::var target;

JUCE_DECLARE_NON_COPYABLE_WITH_LEAK_DETECTOR (Touch)
};

/** Lists used to represent the properties of TouchEvents. */
juce::Array<Touch*> touches;
juce::Array<Touch*> targetTouches;
juce::Array<Touch*> changedTouches;

/** Retrieve a specific Touch object in an array of Touch objects, if it exists. */
static View::Touch* getTouch (const int touchIdentifier, juce::Array<Touch*> myList)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here I'd go for more modern C++ style. So juce::Array has begin() and end() iterators. Meaning we can use std::find_if

{
for (int i = 0; i < myList.size(); ++i)
{
Touch* t = myList.getUnchecked(i);

if (t->identifier == touchIdentifier)
return t;
}

return nullptr;
}

/** Retrieve the index of a specific Touch object in an array of Touch objects, if it exists. */
static int getTouchIndexInList (const int touchIdentifier, juce::Array<Touch*> myList)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

juce::Array has anindexOf function which would remove this boiler plate.

{
for (int i = 0; i < myList.size(); ++i)
{
Touch* t = myList.getUnchecked(i);

if (t->identifier == touchIdentifier)
return i;
}

return -1;
}


protected:
//==============================================================================
juce::NamedValueSet props;
Expand Down
Loading