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.

Sign up for GitHub

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

Jump to bottom

Stamp brush now works with the stagger in hex mode as expected #1496

Merged
merged 9 commits into from
Mar 21, 2017
Merged

Stamp brush now works with the stagger in hex mode as expected #1496

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
1ecedf6
Stamp brush now works with the stager as expected
Bdtrotte Mar 16, 2017
e7a79e6
Style Updates
Bdtrotte Mar 17, 2017
02fb06d
Slight cleanup. Set up to move changes to new branch
Bdtrotte Mar 20, 2017
94a76db
Made requested changes, and general inprovments, style updates
Bdtrotte Mar 21, 2017
2d9d6c8
General inprovments and style updates
Bdtrotte Mar 21, 2017
9814c37
to merge with upsream master
Bdtrotte Mar 21, 2017
b2145f1
No longer makes unnecessary copies!
Bdtrotte Mar 21, 2017
4707b66
Added spaces for style
Bdtrotte Mar 21, 2017
7970270
Last changes!
Bdtrotte Mar 21, 2017
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
6 changes: 6 additions & 0 deletions src/libtiled/map.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -357,6 +357,12 @@ class TILEDSHARED_EXPORT Map : public Object
*/
bool isTilesetUsed(const Tileset *tileset) const;

/**
* Returns whether the map is staggered
*/
bool isStaggered() const
{ return orientation() == Hexagonal || orientation() == Staggered; }

LayerDataFormat layerDataFormat() const
{ return mLayerDataFormat; }
void setLayerDataFormat(LayerDataFormat format)
Expand Down
59 changes: 58 additions & 1 deletion src/tiled/stampbrush.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -250,9 +250,17 @@ void StampBrush::endCapture()

// Intersect with the layer and translate to layer coordinates
QRect captured = capturedArea();

Copy link
Member

Choose a reason for hiding this comment

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

No need to add this line.

captured &= QRect(tileLayer->x(), tileLayer->y(),
tileLayer->width(), tileLayer->height());

//gets if the relative stagger should be the same as the base layer
int staggerIndexOffSet;
if (tileLayer->map()->staggerAxis() == Map::StaggerX)
staggerIndexOffSet = captured.x() % 2;
else
staggerIndexOffSet = captured.y() % 2;
Copy link
Member

Choose a reason for hiding this comment

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

Please move this bit of code down to after if (captured.isValid()) {, since you don't need the variable yet here.


if (captured.isValid()) {
captured.translate(-tileLayer->x(), -tileLayer->y());
Map *map = tileLayer->map();
Expand All @@ -263,6 +271,9 @@ void StampBrush::endCapture()
map->tileWidth(),
map->tileHeight());

stamp->setStaggerAxis(map->staggerAxis());
stamp->setStaggerIndex((Map::StaggerIndex)((map->staggerIndex() + staggerIndexOffSet) % 2));

// Add tileset references to map
foreach (const SharedTileset &tileset, capture->usedTilesets())
stamp->addTileset(tileset);
Expand Down Expand Up @@ -380,13 +391,56 @@ void StampBrush::drawPreviewLayer(const QVector<QPoint> &list)
QRegion paintedRegion;
QVector<PaintOperation> operations;
QHash<TileLayer *, QRegion> regionCache;
QHash<TileLayer *, TileLayer *> shiftedCopies;

for (const QPoint &p : list) {
const TileStampVariation variation = mStamp.randomVariation();
mapDocument()->unifyTilesets(variation.map, mMissingTilesets);

TileLayer *stamp = variation.tileLayer();

Map::StaggerAxis mapStaggerAxis = mapDocument()->map()->staggerAxis();
Map::StaggerIndex mapStaggerIndex = mapDocument()->map()->staggerIndex();
Map::StaggerIndex stampStaggerIndex = variation.map->staggerIndex();


//if staggered map, makes sure stamp stays the same
if (mapDocument()->map()->isStaggered()
&& ((mapStaggerAxis == Map::StaggerY)? stamp->height() > 1 : stamp->width() > 1)) {

if (mapStaggerAxis == Map::StaggerY) {
bool topIsOdd = (p.y() - stamp->height() / 2) & 1;

if ((stampStaggerIndex == mapStaggerIndex) == topIsOdd) {
stamp = variation.tileLayer()->copy(variation.tileLayer()->bounds());
Copy link
Member

Choose a reason for hiding this comment

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

Just use stamp instead of variation.tileLayer(). It's the same thing after all.

shiftedCopies.insert(stamp, stamp);
Copy link
Member

Choose a reason for hiding this comment

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

Before you go and make the copy and insert it, you should of course check whether it isn't already there (meaning as well, that you should use the original stamp as the key, and not the copy). If a shifted copy was already made for this stamp, just assign that to stamp and don't do the shifting again.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Could it be easier to just use a bool flag to see if we've made the copy? As we are only storing one copy. Or is there an advantage to the QHash?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Or for that matter just checking is a TileLayer * == nullptr and if so make the copy, and if not set stamp = copy

Copy link
Member

Choose a reason for hiding this comment

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

No, it's not easier with a bool flag, because you need a bool flag for each variation. This also means we're storing potentially multiple copies (one for each variation that ends up being used). Hence we need the QHash. You see if you've made a copy by doing:

TileLayer *shiftedStamp = shiftedCopies.value(stamp);
if (!shiftedStamp) {
    shiftedStamp = static_cast<TileLayer*>(stamp->clone());
    shiftedCopies.insert(stamp, shiftedStamp);
    // do the shifting
}
stamp = shiftedStamp;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Forgot to take into account the variations! Sorry, still learning tiled and qt as I go!


stamp->resize(QSize(stamp->width() + 1, stamp->height()), QPoint());

for (int y = (stampStaggerIndex + 1) & 1; y < stamp->height(); y += 2) {
for (int x = stamp->width() - 2; x >= 0; --x)
stamp->setCell(x + 1, y, stamp->cellAt(x, y));
stamp->setCell(0, y, Cell());
}
}
} else {
bool leftIsOdd = (p.x() - stamp->width() / 2) & 1;

if ((stampStaggerIndex == mapStaggerIndex) == leftIsOdd) {
stamp = variation.tileLayer()->copy(variation.tileLayer()->bounds());
shiftedCopies.insert(stamp, stamp);

stamp->resize(QSize(stamp->width(), stamp->height() + 1), QPoint());

for (int x = (stampStaggerIndex + 1) & 1; x < stamp->width(); x += 2) {
for (int y = stamp->height() - 2; y >= 0; --y)
stamp->setCell(x, y + 1, stamp->cellAt(x, y));
stamp->setCell(x, 0, Cell());
}
}
}
}

QRegion stampRegion;
if (regionCache.contains(stamp)) {
stampRegion = regionCache.value(stamp);
Expand All @@ -413,8 +467,11 @@ void StampBrush::drawPreviewLayer(const QVector<QPoint> &list)
bounds.x(), bounds.y(),
bounds.width(), bounds.height()));

for (const PaintOperation &op : operations)
for (const PaintOperation &op : operations) {
preview->merge(op.pos - bounds.topLeft(), op.stamp);
}
Copy link
Member

Choose a reason for hiding this comment

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

Coding style: please remove these brackets again now.


qDeleteAll(shiftedCopies);

mPreviewLayer = preview;
}
Expand Down