Fit map in view does not work when tile size is 1

[NworbLegin]

Describe the bug
The "Fit Map In View" function does not work when there is a high tile count. (e.g. 600 x 400 tiles at a tile width height of 8x8)

To Reproduce
Steps to reproduce the behavior:

1. From the menu select File->New->New Map...
2. Set the settings to a tile size of 8px x 8px and a Fixed number of tiles to be Width 600 and Height 400.
3. Press OK
4. The map is initially now shown filling the view
5. Press ctrl+/ or Menu->View->Fit Map In View
6. Observe that the map is not sized to fit in the view

Expected behavior
I would expect that any configuration of map tile size and width/height would be able to be scaled to fit within the view when Fit Map To View is actioned.

Specifications:

• OS: Windows 10 x64
• Tiled Version: 1.9.2 - but I don't think this has ever worked correctly ;-)

[eishiya]

Looking at the code, this behaviour is intentional:

tiled/src/tiled/mapview.cpp

Lines 157 to 165 in 48c4f85

	// Scale and center map to fit in view, while avoiding to go below the
	// minimum scale. For extremely large maps, avoid putting more than about
	// 256 * 256 tiles within the view for performance reasons.
	qreal desiredScale = std::min(width() / rect.width(),
	height() / rect.height()) * 0.95;
	auto tileSize = mMapDocument->map()->tileSize();
	qreal scale256 = std::min(width() / (256.0 * tileSize.width()),
	height() / (256.0 * tileSize.height()));

However, I'd also rather see the map zoomed out, even if that means lag. The main use of Fit Map in View is to deal with large maps, I feel. A performance hit is an acceptable price, especially since users who find this feature not working as expected for large maps are likely to zoom out anyway.

[NworbLegin]

Yep - I would agree that the performance hit is acceptable. Maybe this could be on a user preference setting? "Allow large fit in view"

[NworbLegin]

I have implemented this on a local build and will generate a branch and pull-request when I have time.

[eishiya]

I'm not even sure this needs to be a user preference, it seems like clutter to me. I think users working with large maps expect some performance hitches sometimes, and all this option does is make Fit Map in View not do what it says. I think users for who this performance optimization might make a positive difference would quickly learn not to use Fit Map in View at all :'D

[NworbLegin]

Yep - I'm tempted to agree. Is the Fit Map in View ever called outside of user control? e.g. when opening a map for the first time?

[eishiya]

Not that I can find. fitInView() and fitMapInView() are only called via the action. When creating or opening new maps, Tiled seems to use 100% zoom centred on 0,0.

[NworbLegin]

Okay - Shall I amend my commit request to remove the user settings and make the feature work without restrictions?

[eishiya]

I think it would be good to hear @bjorn's opinion, perhaps there's some reason for the performance mindfulness that we've both overlooked.

[bjorn]

Is the Fit Map in View ever called outside of user control? e.g. when opening a map for the first time?

Yes, in this piece of code:

tiled/src/tiled/mapview.cpp

Lines 461 to 468 in 7fa253f

	if (!mViewInitialized) {
	mViewInitialized = true;
	if (mHasInitialCenterPos)
	forceCenterOn(mInitialCenterPos);
	else
	fitMapInView();
	}

If needed, we can of course differentiate between the automatic version and the manual action, such that only the automatic case has a minimum zoom level.

I would personally also like to avoid adding an option for this.