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

Strange overlapping issue with tilesets using flame_tiled #2619

Closed
invennicoofficial opened this issue Jul 24, 2023 · 24 comments · Fixed by #2701
Closed

Strange overlapping issue with tilesets using flame_tiled #2619

invennicoofficial opened this issue Jul 24, 2023 · 24 comments · Fixed by #2701

Comments

@invennicoofficial
Copy link

invennicoofficial commented Jul 24, 2023

Current bug behavior

There is a strange overlapping issue occurring at the time of rendering the map. I am using embedded tilesets and external as well. I have tried all the encodings(Base64, CSV), but still facing the same. The map is isometric with 256*128 tile size.
Screenshot 2023-07-24 at 11 10 40 AM

Expected behavior

Screenshot 2023-07-24 at 11 11 30 AM

Steps to reproduce

You will need to load isometric tilesets of 256*128 px size. Connect with me if you want the files.

Flutter doctor output

[✓] Flutter (Channel stable, 3.10.6, on macOS 13.4.1 22F770820d darwin-arm64,
    locale en-IN)
    • Flutter version 3.10.6 on channel stable at
      /Users/ayaanpathan/Desktop/Development/FlutterSDK/flutter
    • Upstream repository https://github.com/flutter/flutter.git
    • Framework revision f468f3366c (11 days ago), 2023-07-12 15:19:05 -0700
    • Engine revision cdbeda788a
    • Dart version 3.0.6
    • DevTools version 2.23.1

[✓] Android toolchain - develop for Android devices (Android SDK version 33.0.0)
    • Android SDK at /Users/ayaanpathan/Library/Android/sdk
    • Platform android-33, build-tools 33.0.0
    • Java binary at: /Applications/Android
      Studio.app/Contents/jbr/Contents/Home/bin/java
    • Java version OpenJDK Runtime Environment (build
      17.0.6+0-17.0.6b802.4-9586694)
    • All Android licenses accepted.

[✓] Xcode - develop for iOS and macOS (Xcode 14.3.1)
    • Xcode at /Applications/Xcode.app/Contents/Developer
    • Build 14E300c
    • CocoaPods version 1.12.0

[✓] Chrome - develop for the web
    • Chrome at /Applications/Google Chrome.app/Contents/MacOS/Google Chrome

[✓] Android Studio (version 2022.2)
    • Android Studio at /Applications/Android Studio.app/Contents
    • Flutter plugin can be installed from:
      🔨 https://plugins.jetbrains.com/plugin/9212-flutter
    • Dart plugin can be installed from:
      🔨 https://plugins.jetbrains.com/plugin/6351-dart
    • Java version OpenJDK Runtime Environment (build
      17.0.6+0-17.0.6b802.4-9586694)

[✓] VS Code (version 1.76.2)
    • VS Code at /Applications/Visual Studio Code.app/Contents
    • Flutter extension version 3.60.0

[✓] Connected device (2 available)
    • macOS (desktop) • macos  • darwin-arm64   • macOS 13.4.1 22F770820d
      darwin-arm64
    • Chrome (web)    • chrome • web-javascript • Google Chrome 114.0.5735.198

[✓] Network resources
    • All expected network resources are available.

• No issues found!

More environment information

Create a list of more environment information, like:

  • Flame version: 1.8.0
  • Flame tiled version: 1.12.0
  • Platform affected: android, ios, web
  • Platform version affected: tested on android 12, iOS 16 and web

Log information

No errors

Upvote & Fund

  • We're using Polar.sh so you can upvote and help fund this issue.
  • We receive the funding once the issue is completed & confirmed by you.
  • Thank you in advance for helping prioritize & fund our backlog.
Fund with Polar
@invennicoofficial
Copy link
Author

@spydon any idea on why this behaviour is happening?

@spydon
Copy link
Member

spydon commented Jul 27, 2023

@spydon any idea on why this behaviour is happening?

No idea, I've barely used tiled.
Maybe @ufrshubham @jtmcdole or @Hwan-seok knows?

@invennicoofficial
Copy link
Author

Thanks @spydon have you worked with isometric maps before? if so, any tool you can suggest which will work for generating readable formats for flame?

@spydon
Copy link
Member

spydon commented Jul 28, 2023

Thanks @spydon have you worked with isometric maps before? if so, any tool you can suggest which will work for generating readable formats for flame?

I have only used the IsometricTileMapComponent, so not anything where you externally generate the map.

@invennicoofficial
Copy link
Author

Thanks @spydon

@Hwan-seok
Copy link
Contributor

@invennicoofficial
Could you send your map resources to tttkhs96@gmail.com?
I can't affirm the problem will be discovered but I will give it a try.

@invennicoofficial
Copy link
Author

Absolutely @Hwan-seok Thank you!!

@invennicoofficial
Copy link
Author

@Hwan-seok done.

@jtmcdole
Copy link
Contributor

Late to the game. What is the tile sizes here? They don't look correct even in the Tiled editor shot you have.

@chippydip
Copy link
Contributor

chippydip commented Aug 1, 2023

I just realized this may be the same issue I just reported (#2633) with a fix (#2634) . Is the grid size smaller than your tile size? If so, I believe the scale calculation is incorrect in that case.

@invennicoofficial
Copy link
Author

Tile size is 256*128

@invennicoofficial
Copy link
Author

This is my map properties

Screenshot 2023-08-01 at 12 14 20 PM

@chippydip
Copy link
Contributor

In the Tiled screenshot it looks like each tile is much larger than the map's grid size. Are all of those building individual 256x128 tiles stitched together or is the school (for example) one big ~2560x1280 single tile image that covers multiple grid cells?

@Hwan-seok
Copy link
Contributor

Hwan-seok commented Aug 1, 2023

It seems fine with Flutter 3.7.10,

But it crashes after upgrading Flutter to 3.10.xx with exactly the same code and dependencies🤔🤔🤔🤔🤔
=> Only IOS, probably it is a different one from this issue(impeller)

I couldn't reproduce your problem with your map resources and the same versions. Is there something I missed it?
Here's my code:

class AppGame extends FlameGame {
  final world = World();
  late final CameraComponent cameraComponent;

  @override
  Future<void>? onLoad() async {
    cameraComponent = CameraComponent(world: world);
    addAll([cameraComponent, world]);

    final contents = await Flame.bundle.loadString(
      'assets/tiles/ymcaNew.tmx',
    );
    final tiledMap = await TiledMap.fromString(
      contents,
      FlameTsxProvider.parse,
    );

    world.add(
      TiledComponent(
        await RenderableTiledMap.fromTiledMap(
          tiledMap,
          Vector2(256, 128),
        ),
      ),
    );
    cameraComponent.viewfinder.position = Vector2(5000, 1000);
    cameraComponent.viewfinder.zoom = 0.1;

    return super.onLoad();
  }
}

@invennicoofficial
Copy link
Author

In the Tiled screenshot it looks like each tile is much larger than the map's grid size. Are all of those building individual 256x128 tiles stitched together or is the school (for example) one big ~2560x1280 single tile image that covers multiple grid cells?

@chippydip Yes, all are individual 256X128 tiles stitched together, we've created multiple layers and tile set with each buildings

@invennicoofficial
Copy link
Author

@Hwan-seok thanks for your snippet, we've tried to same way again with same file but buildings are still overlapping.

@invennicoofficial
Copy link
Author

@spydon @Hwan-seok @chippydip I've tried to implement the same tmx file into unity game engine and it's working fine but loading same tmx file into flame, I'm facing issue of overlapping the two buildings.

Can you please help me here with solution so I can quickly apply into the game.

  • Unity Game Screenshot
    image

Note: Firstly, I was getting error of smaller size sprite on unity but once I've update the max size of sprite, the error resolved and works fine on unity.

image

@polar-sh polar-sh bot added the polar label Aug 17, 2023
@ufrshubham
Copy link
Collaborator

Okay, so I've looked in this problem and it seem like this is happening due to the size limits we use in RectangleBinPacker for web.

this.maxX = kIsWeb ? 4096 : 8192,
this.maxY = kIsWeb ? 4096 : 8192,

Following comment explains why we choose those limits,

/// Note: Chrome on Android has a maximum texture size of 4096x4096. kIsWeb is
/// used to select the smaller texture and might overflow. Consider using
/// smaller textures for web targets, or, pack your own atlas.

but can't remember if the problem was with just web builds running on Android or was it for web builds on desktop too. If it was just for web-Android, then probably we should check for platform along with KIsWeb before reducing the size.


From the files shared by @invennicoofficial, I was able to reproduce the problem and can confirm that the bin packer overflows and starts placing the new rects at the top left. This happens because we return Rect.zero when the packer is filled.

// If we exhausted our search, return an empty rect.
if (search == null || search.width < width || search.height < height) {
return Rect.zero;
}

I think it would be better if the packer could report that it is overflowing instead of silently overlapping the images. It will at least help the users understand why they are seems the wrong output.

@jtmcdole a question for you:
Is it absolutely necessary to have a single atlas for the whole tilemap? IIRC, this packing was done to optimize the rendering. If so, can't we create a new one once the previous one starts overflowing?

Also, I noticed that we pack all the images from all the tilesets of a map. But it is possible that some tilesets is not event used in the tilemap. This unnecessarily makes the packer run out of space due to images from unused tilesets. Not sure if it is even possible to detect this, but I experienced this problem with embeded tilesets. Even doing a proper export from Tiled wasn't able to get rid of the unused tilesets.

@jtmcdole
Copy link
Contributor

@ufrshubham

  1. Texture size limit: This is just Chrome / WebView on Android. It was hard coded... however if I look at the gpu_driver_bug_list_json.cc and Linux MESA (it use to also include Mac+Intel GPUs and Mac+AMD Gpus; so consider it fickle). Flutter needs to give us the MAX_TEXTURE_SIZE parameter from the GPU.

  2. Silently Overlapping: I had a comment about that in the original CL and if I recall correctly, we didn't want to have the program blowing up. I believe I had it throw before.

  3. It is required to have a single atlas because it stacks all the draw calls into a single paint. There is probably space left in the rect; 2D bin packing is NP-Complete. You'll at least know if it can be done at all by adding up the area of all the rects being packed and seeing if its less than the 4k rect.

If you make two rects to pack, you'll get back to painting over yourself and having misordering unless we add logic to interleave drawing between, like: [ atlas2 {rects...} , atlas1 {rects...} , atlas2 {rects...} ] in a single frame draw.

  1. It is not known at packing time if a texture will be used or not. You can dynamically use images later, which would be a miss if its not in the packing.

So questions are:

  • How big is the tileset being used?
  • Are any of the tiles being flipped? This automatically increases the size of the map.
  • Can it be tiled ahead of time? (would give you ultimate control of the sprite map packing)
  • Do we want to have a fallback to an alternative rendering format with multiple atlas?

@ufrshubham
Copy link
Collaborator

  • How big is the tileset being used?

In the example that they shared, each tileset was made up of a single image and average size of images was around 1000x1000. The total packed size was turning out to be 6885×3274.

  • Are any of the tiles being flipped? This automatically increases the size of the map.

No.

  • Can it be tiled ahead of time? (would give you ultimate control of the sprite map packing)

This is one solution that I've suggested them to try. But looking at their image dimensions, I think they'll have to create multiple atlas to keep everything under 4k.

  • Do we want to have a fallback to an alternative rendering format with multiple atlas?

Only if we start getting more of such usecases from users. For pixel art style games I don't see us hitting these limits that often.

@jtmcdole
Copy link
Contributor

6885×3274 <- dang. So they have more than 16 1k image tiles.

If the maps don't overlap; they could make multiple maps with the different tilesets and line them up in game. If we had a fallback then this wouldn't be a problem, but they would start running into performance issues. Other solutions that might speed things up is:

  • camera culling rects
  • maybe back-buffer moving (if the mape is sliding up/down left/right, we could just re-paint the previous canvas... not sure we have access to that in flutter.)

In a prior life I worked on really low end hardware. I would do rect collisions from the base up to limit the number of full screen repaints; today we're still doing full screen repaints in Flame/Flutter.

@ufrshubham
Copy link
Collaborator

Just to close the loop, here is what I think can be done for now:

  • Add an assert or debugPrint to let the users know about the overflow. Bluefire team can take a call on this.
  • If possible add some way to let the users override the maxX and maxY. This will allow them the freedom to use custom sized packer based on their target platform and hardware.

@spydon
Copy link
Member

spydon commented Aug 31, 2023

Just to close the loop, here is what I think can be done for now:

  • Add an assert or debugPrint to let the users know about the overflow. Bluefire team can take a call on this.
  • If possible add some way to let the users override the maxX and maxY. This will allow them the freedom to use custom sized packer based on their target platform and hardware.

Both good suggestions that would be implemented I think!

@jtmcdole
Copy link
Contributor

jtmcdole commented Sep 5, 2023

If possible add some way to let the users override the maxX and maxY. This will allow them the freedom to use custom sized packer based on their target platform and hardware.

We can do this with environment variables. You will have a bad time on systems that don't support max{XY} though. This information is AVAILABLE at a webgl layer... but I don't think it is exposed by Flutter. We could write a silly non-flutter, webgl library that checks and returns this information?

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

Successfully merging a pull request may close this issue.

6 participants