"SuperFastAndUgly"It is actually possible to change the interpolation done for the aspect ratio correction in SDL graphics modes by changing this line of code (for example using kSuperFastAndUglyAspectMode to get nearest neighbor interpolation).
Beauty is in the eye of the beholder, as they say. . . Take a look at these two images: (download, the preview is too blurry)
FoA_ScummVM_1x_SDL_VeryFastAndGood.png: https://drive.google.com/open?id=1p2wpL ... 8sKlz3INTV
FoA_ScummVM_1x_SDL_SuperFastAndUgly.png: https://drive.google.com/open?id=1_TWAb ... MMzcPYKKHo
Which one was ugly again?
This was a worst-case scenario to illustrate how bad the "smearing" is potentially, at 1x and with the verb boxes in SCUMM. With nearest neighbor it is a bit mangled, but with bilinear it is just unspeakable.
kVeryFastAndGoodAspectMode, kFastAndVeryGoodAspectMode, kSlowAndPerfectAspectMode are all functionally the same. Not so "slow and perfect" after all, as it turns out.
I quickly added a 4x scaler myself for testing purposes, and ironically, at native 4x and at native 3x to a lesser extent, the dirty scaling doesn't look that terrible. I initially came upon this problem with the scaling artifacts that appear when you then scale to arbitrary resolutions from the initial graphics mode scaling.
This is the reason why this bug was never found or acknowledged. Up until recently you couldn't even change the size of the window manually in SDL, so anyone who didn't force scaling in drivers would never imagine there was a problem, like I didn't until now.
You said
Well, I'm not sure about that dude. Look at this: (you can tell from the preview, but download if you don't believe me; they are the same)However this is not going to be perfect due to image being scaled in two passes.
KQV_DOSBox_4x_SDL_surfacenb.png: https://drive.google.com/open?id=1oKFEp ... EA9ny4wf7r
KQV_ScummVM_4x_SDL_SuperFastAndUgly.png: https://drive.google.com/open?id=1iH3RT ... oZgSEAj94f
Unless DOSBox *also* has this two-pass scaling thing, there doesn't seem to be a problem.
I actually traced this whole bilinear filtering thing back to the original commit, which was in fact the original commit to introduce aspect ratio correction itself in June of 2003 (!): https://github.com/scummvm/scummvm/comm ... c130f81af0
It came with a comment about nearest neighbor: a little bit faster, but looks ugly
I don't know if that made sense at the time (probably not), but looking at the git history, that meme seems to have lived on in various forms throughout the years . . .
. . . all the way to the current year with the macros:
Code: Select all
#define kSuperFastAndUglyAspectMode 0 // No interpolation at all, but super-fast
#define kVeryFastAndGoodAspectMode 1 // Good quality with very good speed
#define kFastAndVeryGoodAspectMode 2 // Very good quality with good speed
#define kSlowAndPerfectAspectMode 3 // Accurate but slow code
I will stop my rant now at the risk of angering some well-meaning developer. I could instead address the elephant in the room here: Was the aspect ratio scaling effectively bugged like this during all these years? Also, is that two-pass scaling still a problem like you said, criezy? (ScummVM SDL versus DOSBox SDL still look a tiny bit different on lower resolutions).
Here are several more screenshots for various comparisons: https://drive.google.com/open?id=1lQmoW ... IJwg7rzRet
and here is my suggested fix: Delete most of the blocks and basically anything that says "bilinear" from graphics/scaler/aspect.cpp. criezy, if you or anyone can try this:
Code: Select all
/* ScummVM - Graphic Adventure Engine
*
* ScummVM is the legal property of its developers, whose names
* are too numerous to list here. Please refer to the COPYRIGHT
* file distributed with this source distribution.
*
* This program is free software; you can redistribute it and/or
* modify it under the terms of the GNU General Public License
* as published by the Free Software Foundation; either version 2
* of the License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with this program; if not, write to the Free Software
* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
*
*/
#include "graphics/scaler/intern.h"
#include "graphics/scaler/aspect.h"
#ifdef USE_ARM_NEON_ASPECT_CORRECTOR
#include <arm_neon.h>
#endif
void makeRectStretchable(int &x, int &y, int &w, int &h) {
/*
#if ASPECT_MODE != kSuperFastAndUglyAspectMode
int m = real2Aspect(y) % 6;
// Ensure that the rect will start on a line that won't have its
// colors changed by the stretching function.
if (m != 0 && m != 5) {
y -= m;
h += m;
}
#if ASPECT_MODE == kVeryFastAndGoodAspectMode
// Force x to be even, to ensure aligned memory access (this assumes
// that each line starts at an even memory location, but that should
// be the case on every target anyway).
if (x & 1) {
x--;
w++;
}
// Finally force the width to be even, since we blit 2 pixels at a time.
// While this means we may sometimes blit one column more than necessary,
// this should actually be faster than having the check for the
if (w & 1)
w++;
#endif
#endif
*/
}
/**
* Stretch a 16bpp image vertically by factor 1.2. Used to correct the
* aspect-ratio in games using 320x200 pixel graphics with non-qudratic
* pixels. Applying this method effectively turns that into 320x240, which
* provides the correct aspect-ratio on modern displays.
*
* The image would normally have occupied y coordinates origSrcY through
* origSrcY + height - 1.
*
* However, we have already placed it at srcY - the aspect-corrected y
* coordinate - to allow in-place stretching.
*
* Therefore, the source image now occupies Y coordinates srcY through
* srcY + height - 1, and it should be stretched to Y coordinates srcY
* through real2Aspect(srcY + height - 1).
*/
template<typename ColorMask>
int stretch200To240(uint8 *buf, uint32 pitch, int width, int height, int srcX, int srcY, int origSrcY) {
int maxDstY = real2Aspect(origSrcY + height - 1);
int y;
const uint8 *startSrcPtr = buf + srcX * 2 + (srcY - origSrcY) * pitch;
uint8 *dstPtr = buf + srcX * 2 + maxDstY * pitch;
for (y = maxDstY; y >= srcY; y--) {
const uint8 *srcPtr = startSrcPtr + aspect2Real(y) * pitch;
if (srcPtr == dstPtr)
break;
memcpy(dstPtr, srcPtr, sizeof(uint16) * width);
dstPtr -= pitch;
}
return 1 + maxDstY - srcY;
}
int stretch200To240(uint8 *buf, uint32 pitch, int width, int height, int srcX, int srcY, int origSrcY) {
extern int gBitFormat;
if (gBitFormat == 565)
return stretch200To240<Graphics::ColorMasks<565> >(buf, pitch, width, height, srcX, srcY, origSrcY);
else // gBitFormat == 555
return stretch200To240<Graphics::ColorMasks<555> >(buf, pitch, width, height, srcX, srcY, origSrcY);
}
template<typename ColorMask>
void Normal1xAspectTemplate(const uint8 *srcPtr, uint32 srcPitch, uint8 *dstPtr, uint32 dstPitch, int width, int height) {
for (int y = 0; y < (height * 6 / 5); ++y) {
if ((y % 6) == 5)
srcPtr -= srcPitch;
memcpy(dstPtr, srcPtr, sizeof(uint16) * width);
srcPtr += srcPitch;
dstPtr += dstPitch;
}
}
void Normal1xAspect(const uint8 *srcPtr, uint32 srcPitch, uint8 *dstPtr, uint32 dstPitch, int width, int height) {
extern int gBitFormat;
if (gBitFormat == 565)
Normal1xAspectTemplate<Graphics::ColorMasks<565> >(srcPtr, srcPitch, dstPtr, dstPitch, width, height);
else
Normal1xAspectTemplate<Graphics::ColorMasks<555> >(srcPtr, srcPitch, dstPtr, dstPitch, width, height);
}
#ifdef USE_ARM_SCALER_ASM
extern "C" void Normal2xAspectMask(const uint8 *srcPtr,
uint32 srcPitch,
uint8 *dstPtr,
uint32 dstPitch,
int width,
int height,
uint32 mask);
/**
* A 2x scaler which also does aspect ratio correction.
* This is Normal2x combined with vertical stretching,
* so it will scale a 320x200 surface to a 640x480 surface.
*/
void Normal2xAspect(const uint8 *srcPtr,
uint32 srcPitch,
uint8 *dstPtr,
uint32 dstPitch,
int width,
int height) {
extern int gBitFormat;
if (gBitFormat == 565) {
Normal2xAspectMask(srcPtr,
srcPitch,
dstPtr,
dstPitch,
width,
height,
0x07e0F81F);
}
else {
Normal2xAspectMask(srcPtr,
srcPitch,
dstPtr,
dstPitch,
width,
height,
0x03e07C1F);
}
}
#endif // USE_ARM_SCALER_ASM
