Added tools to create and modify bezier objects

[zilluss]

The following things are still missing:

• Icons for the toolbar
• Shortcuts for the "create bezier*" tools
• Translations
• Export to formats other than JSON

[bjorn]

Translations

They will come with the string freeze for 0.11, no need for that here.

Export to formats other than JSON

It seems you've actually already implemented TMX as well. That only leaves the Lua format, which can be added later.

About the storage format though, I had based the polygon and polyline elements on the SVG specification and thought it may be good to do the same here. That would mean saving this to a path element, for example a simple cubic curve:

<path d="M0,0 C0,100 100,100 100,0"/>

This means, first move to position 0,0 (these coordinates would be relative to the object's position), then do a cubic curve to 100,0 using 0,100 and 100,100 as the two control points.

This format does not seem overly hard to parse to me nor should it be a problem to write it out. Of course, we do not need to support the full standard immediately but extending the support later would anyway be a possibility.

While trying out the patch I found some problems:

• Clickable regions for selecting / moving bezier objects seemed to be not working properly.
• The control points and their lines are not positioned correctly when objects are rotated.
• Lines to the control points are not painted correctly when they extend beyond the bounding box of the object.
• When moving bezier object nodes, I would expect their control points to move along with them.
• Objects complete change shape when flipped along their X or Y axis (actions available in the context menu).

Overall though, this is a really promising feature and I think you've done a good job!

[bjorn]

Icons for the toolbar

I've created the others and can add some in similar style for the new tools. It's basically just taking that arrow and adding the shape as it appears in Tiled itself.

[zilluss]

I was expecting that a few things might not work correctly, and some problems were introduced when I added the undo/split/delete/join functionalities. I'll fix the problems you've mentioned.

[zilluss]

The previous problems have been fixed. I would appreciate it if someone could test the new version if he or she spots any new problems I didn't catch.

[bjorn]

Thanks for the fixes! These two problems I wrote before remain:

• Clickable regions for selecting / moving bezier objects seemed to be not working properly.
• Lines to the control points are not painted correctly when they extend beyond the bounding box of the object.

[zilluss]

• This problem seems to occur when the bezier segment is straight. In that case, the mMapScene->items(pos) call returns no object. The problem can be alleviated by using a rectangle instead of a point to find the items under the mouse, but this affects all the other object too.
• I didn't notice the problem with the control point lines, because it only happens when OpenGL acceleration is turned off. I'm currently investigating the issue.

[zilluss]

Unfortunately, the commit was immediately pushed.

[bjorn]

Unfortunately, the commit was immediately pushed.

Well, you can still squash the commit locally into the previous one and force-push the new version to your beziertool branch. If you're on your local beziertool branch, you would do git rebase -i HEAD^^, change the second line to have the word fixup instead of pick at the start, save and close the editor. Of course, if you're new to this you should be extra careful to understand what you're doing.

[zilluss]

I usually don't rewrite history out of principle, but I understand that you want a clean history and squashed the commit.

[bjorn]

I usually don't rewrite history out of principle, but I understand that you want a clean history and squashed the commit.

Thanks! I also try to avoid rewriting history, but I don't consider changing something that is still work-in-progress as rewriting history. This is where history gets created, and every effort should be made that it is comfortable to look back on it. :-)

[Darky-Lucera]

Sorry, now that you have support for bezier curves, what about adding support to create other kind of splines without control points?

I think theese kind of splines are more easy to control because they pass smoothly, or tight/tense if you want, through all your vertices (because there are no control points).

Some kind of splines without control points:

• Finite or Catmull–Rom: Easy ones, no params. http://en.wikipedia.org/wiki/Cubic_Hermite_spline#Catmull.E2.80.93Rom_spline http://en.wikipedia.org/wiki/Cubic_Hermite_spline#Finite_difference
• Cardinal: Something in the middle, one param. http://en.wikipedia.org/wiki/Cubic_Hermite_spline#Cardinal_spline Note that with tension = 1 you would have the current tight/tense polygons (it must be a little change).
• TCB: The most complicated, three params. http://en.wikipedia.org/wiki/Cubic_Hermite_spline#Kochanek.E2.80.93Bartels_spline

Once that has been programmed one type of theese spline the others are easy because you only must change the calculus of the tangents.

Maybe the big problem here is how to select the desired spline in the editor. Perhaps using some kind of button like ones in the tool window of photoshop.

Personally I can not add these features because I don't know QT and I don't have experience with this project, which must be huge. Although I can give you C++ code that implement these kind of splines.

Thanks in advance!

[bjorn]

@Darky-Lucera That definitely sounds interesting, but whether it can be done or not basically depends on what the QPainter API allows us to write since Tiled does not do its own rasterization. It seems you know quite a bit about this so maybe you could have a look at the API to check whether it could enable these kinds of curvers: http://qt-project.org/doc/qt-5/qpainter.html

Edit: Actually the most relevant parts are probably in QPainterPath: http://qt-project.org/doc/qt-5/qpainterpath.html

[Darky-Lucera]

Sure, we can do it because we can draw a line, and we can aproximate any continuous curve drawing little lines. In this case using moveTo and lineTo.

Furthermore we can draw closed spline curves (is similar to draw polygons instead of polylines).

[bjorn]

@Darky-Lucera Ok, I worry a little about the performance and the quality in that case, but sure it could be done if you manage to find the right number of lines to draw. Anyway, it's a different issue so let's not further pollute this pull request. :-)

The support for bezier curves is not actually in yet, but I am having trouble finding time to get around to another round of testing and code review. Also, I'm not that happy with the way the data is stored, on the C++ side as well as in the TMX format (see my first comment about that). So either @zilluss or me shall have to consider alternatives and possibly implement them. Due to these concerns I'm also leaning towards first releasing Tiled 0.11 with the hexagonal map feature before merging this in.

[Darky-Lucera]

It saddens me to hear this but you are the project manager ;)

I think tiled is not a CAD/CAM program, it's just a wonderful tool to make maps for our games, so the quality of the curve will be enough (we must adjust this quality in our games, here we need to see an aproximation about how it will look), and nowadays you will not worry about the performance of drawing some tens (or hundreds, or thousands) of lines to aproximate a curve.

With respect to the format, I would prefer something like that:

<object name="cell" type="membrane" x="3104" y="896">
  <path type="bezier" points="0,0 1,1 ..." controlPoints="0,1 1,2 ..." />
  or
  <path type="cardinal" points="0,0 1,1 ..." tensions="0,1,..." />
  or
  <path type="tcb" points="0,0 1,1 ..." tensions="0,1,..." continuities="..." biases="..." />
</object>

Do we create a new feature request to add support for splines?

[zilluss]

@Darky-Lucera I haven't had QT experience before this PR either :smile:. Having looked at the other kind of splines, they would definitely be a nice addition and easier to implement than beziers, because without control points you can basically reuse most of the polygon code. As for the format, Thorbjørn wants to stick with the SVG standard which probably makes the more sense for most users.

@bjorn I've had a quick glance over the QT source for the beziers and as far as I can tell, QT splits the bezier into lines as well. Also, during debugging I noticed that QT is rather lazy when it comes to redrawing so I think they probably use some kind of offscreen buffer for the paths. I will change the TMX representation when I find the time. I'd schedule the change of the internal representation for a later refactoring, but that's up to you to decide.

[bjorn]

Internal and external data storage aside, I can't merge this pull request mainly because there are still click area issues. Especially for the bezierloop I'm having trouble finding the clickable areas when trying to drag around the objects. This is a problem that needs to be looked into. Also I still see repainting issues when dragging around the control point handles, but that is a more minor issue.

[Darky-Lucera]

If you generate a polygon for the curve, that I think will be faster to draw, you can easily solve the click area issue. This algorithm is known for many years: http://www.ecse.rpi.edu/Homepages/wrf/Research/Short_Notes/pnpoly.html

Here is the code, for reference. Excluding lines with only braces, there are only 7 lines of code.

int pnpoly(int nvert, float *vertx, float *verty, float testx, float testy)
{
  int i, j, c = 0;
  for (i = 0, j = nvert-1; i < nvert; j = i++) {
    if ( ((verty[i]>testy) != (verty[j]>testy)) &&
     (testx < (vertx[j]-vertx[i]) * (testy-verty[i]) / (verty[j]-verty[i]) + vertx[i]) )
       c = !c;
  }
  return c;
}

• W. Randolph Franklin

Enjoy it!

[zilluss]

No problem, I'll fix it when I take care of the click area issue. I hope I'll get to fix it soon.

[TangentFoxy]

So how close or not close to being added to Tiled is this? I didn't realize this wasn't a feature already and need it for a project I was considering Tiled for. :/

[bjorn]

So how close or not close to being added to Tiled is this? I didn't realize this wasn't a feature already and need it for a project I was considering Tiled for. :/

Two years ago I planned to get around this, but in the end I wanted to get Tiled 1.0 out and this feature wasn't part of that plan. This pull request was never in a finished state and I think substantial work is still required to get it done.

Right now, Tiled 1.0 is really around the corner (I plan to release it towards the end of next month) and after that it is time to think about how to prioritize feature development for 1.1, 1.2, etc. This is certainly one of the features I'd like to get around to, but so are many others and I just have 2 full days per week for Tiled development (which aren't fully funded yet). In addition, in June-August we'll be mentoring several (hopefully) students working on Tiled as part of the Google Summer of Code, but there is no project related to this feature specifically.

So short answer is, normally this feature could be somewhere between 4-8 months away. At best you could become a major patron and I could prioritize it right after the Tiled 1.0 release. But I could imagine a better alternative for you could be to rely on another tool, like Inkscape.

[TangentFoxy]

Inkscape doesn't do what I need. I'll be making my own map editor to solve the problem (I needed an excuse to work on my GUI lib anyhow). I'd love to fund the development, but I'm a poor student haha.

Thank you for the detailed reply and explanation!

[IoriBranford]

Do you guys remember what's missing on this? Maybe a checklist so more of us could help out.

[bjorn]

Do you guys remember what's missing on this? Maybe a checklist so more of us could help out.

Please read up on the discussion above. I think there are still clickability issues and the storage format comments also still apply. Besides that, the changes should be rebased onto the latest master branch to resolve the conflicts.

[zilluss]

I will have a look at it this week.

[bjorn]

I will have a look at it this week.

I've afraid this code has changed quite a lot since you made this pull request, but it's great that you'd like to update it! Feel free to ask questions if you're confused by some of the conflicts.

[zilluss]

Yeah I tried a merge, but it's simpler to just look at the current master and the changes I made and re-do the changes on top of the current master. It's surprisingly little code anyway.