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

tiledquickplugin: only render visible tiles #2772

Merged
merged 5 commits into from
Mar 18, 2020

Conversation

mitchcurtis
Copy link
Contributor

Fixes #2769

mitchcurtis and others added 3 commits March 17, 2020 09:57
…ing callback

This allows removing some duplicated code introduced in the previous
commit, and also:

"This could even help enabling export of isometric maps to GameMaker 1.4 format,
which needs to write out XML elements representing each tile."
Cleaned up the code such that the new approach is now used for all
map orientations.

The small optimization that was used for orthogonal tile layers was
dropped for the sake of simplicity. It is not expected to make much of
an impact.
@mitchcurtis
Copy link
Contributor Author

mitchcurtis commented Mar 18, 2020

Nice, thanks! All works great now.

I just noticed that I have some leftovers in src/tiledquick/qml/main.qml. Will these changes be squashed and so I should just do a fixup commit? Or should I rebase 9f2d7b0 (and I guess force push)?

@bjorn
Copy link
Member

bjorn commented Mar 18, 2020

Yeah, that was a nice cleanup in Tiled Quick and now all orientations are supported. :-)

I wonder a bit about the impact on performance of adding a std::function call for each tile, but I did not measure it. A debug build is slow for big maps, but that was already the case. In release mode I did not notice any difference.

I just noticed that I have some leftovers in src/tiledquick/qml/main.qml. Will this changes be squashed and so I should just do a fixup commit? Or should I rebase 9f2d7b0 (and I guess force push)?

Let's squash this, so please push a cleanup commit. :-)

@mitchcurtis
Copy link
Contributor Author

I wonder a bit about the impact on performance of adding a std::function call for each tile, but I did not measure it. A debug build is slow for big maps, but that was already the case. In release mode I did not notice any difference.

I figured it was the lesser of two evils, but that was because I was assuming that using an interface would require memory to be allocated... thinking about it a bit more now, passing a helper object thingy that was created on the stack should also work.

@bjorn
Copy link
Member

bjorn commented Mar 18, 2020

I figured it was the lesser of two evils, but that was because I was assuming that using an interface would require memory to be allocated... thinking about it a bit more now, passing a helper object thingy that was created on the stack should also work.

The std::function wrapping a lambda likely takes pretty much the same amount of memory as the helper object would (though indeed it may be heap allocated rather than stack allocated, depending on how big it is). Also, the helper object would need a virtual member function, so it's likely to have similar overhead to the std::function. But would need to be measured to be sure. :-)

I think the only way to have really less overhead would be to use a template parameter, but it would mean moving the entire tile layer rendering code into the header as well as to remove the virtual dispatch for drawTileLayer.

@bjorn bjorn merged commit d3badef into mapeditor:master Mar 18, 2020
@mitchcurtis
Copy link
Contributor Author

mitchcurtis commented Mar 20, 2020

I've noticed that maps are now shifted half a tile upwards. Do you have any idea which part of our recent changes could be causing this?

diff --git a/src/tiledquick/qml/main.qml b/src/tiledquick/qml/main.qml
index d67909ea..5ddfdff6 100644
--- a/src/tiledquick/qml/main.qml
+++ b/src/tiledquick/qml/main.qml
@@ -151,6 +151,12 @@ ApplicationWindow {
                             mapView.width / scale,
                             mapView.height / scale);
                 }
+
+                Rectangle {
+                    anchors.fill: parent
+                    color: "transparent"
+                    border.color: "darkorange"
+                }
             }
         }
     }

Screen Shot 2020-03-20 at 5 23 48 pm

@bjorn
Copy link
Member

bjorn commented Mar 22, 2020

I've noticed that maps are now shifted half a tile upwards. Do you have any idea which part of our recent changes could be causing this?

So, actually only maps using that particular tileset would appear shifted like that. And it's because the tiles are 64 pixels high and have a transparent region at the bottom. In addition, our tile rendering callback is not applying the drawing offset of the tileset. I'll fix that up now. :-)

bjorn added a commit that referenced this pull request Mar 22, 2020
@mitchcurtis
Copy link
Contributor Author

Thank you Bjorn! Now I can start making some bigger maps! :D

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Tiled Quick: rendering only visible tiles in isometric maps
2 participants