| Re: [hatari-devel] defaulting to SMALL_MEM |
[ Thread Index |
Date Index
| More lists.tuxfamily.org/hatari-devel Archives
]
Hi,
On 15.2.2022 21.01, Thomas Huth wrote:
Am Sun, 13 Feb 2022 19:10:44 +0200
schrieb Eero Tamminen <oak@xxxxxxxxxxxxxx>:
I.e. if screen pointer would be in such RAM area, should the other
video related counters still change or not?
Well, likely yes, but in this case I wonder if that'd buy us much. It
certainly would make the current ST-low rendering code (which is
already quite complex) way more complicated (and likely slower) if we
add checks there all over the place. And for what? As long as we don't
know of any program that relies on such weird behavior, it does not
have any benefit for us, does it?
For the mono function it's trivial, but I hadn't realized how much
things would need to be changed or the color one before actually looking
into it.
Attached is patch handling all the memcpy() cases,
which is already quite large. However, that is missing all the
additional word accesses through pVideoRasterEndLine pointer, used for
STE special cases, which would be even trickier to handle.
=> I concur that it's not really workable for Video_CopyScreenLineColor().
(That function really is one scary monster. Would be nice if somebody
were able to split it into more manageable smaller functions.)
And should there be any additional exceptions happening if screen
pointer is set to non-existing RAM?
I don't think so. Apart from the CPU, none of the other chips can
trigger a bus error, as far as I know.
And does that behavior differ between different Atari machines and
memory addresses?
As we recently learnt, the SCSI DMA controller in the TT can at least
detect bus errors and signal that via a bit. But I don't think that
this will trigger a normal bus error exception on the CPU.
Ok, thanks for the verification!
- Eero
diff --git a/src/video.c b/src/video.c
index a80fd87c..1de86b60 100644
--- a/src/video.c
+++ b/src/video.c
@@ -487,6 +487,8 @@ const char Video_fileid[] = "Hatari video.c";
#define BORDERMASK_NO_SYNC 0x4000
#define BORDERMASK_SYNC_HIGH 0x8000
+#define OUTSIDE_RAM(x) ((x) > &STRam[STRamEnd])
+
//#define STF_SHORT_TOP
int STRes = ST_LOW_RES; /* current ST resolution */
@@ -3409,21 +3411,18 @@ static void Video_StoreResolution(int y , bool start)
*/
static void Video_CopyScreenLineMono(void)
{
- if (pVideoRaster + SCREENBYTES_MONOLINE > &STRam[STRamEnd])
- {
- memset(pSTScreen, 0, SCREENBYTES_MONOLINE);
- pSTScreen += SCREENBYTES_MONOLINE;
- return;
- }
-
+ bool offram = OUTSIDE_RAM(pVideoRaster + SCREENBYTES_MONOLINE);
/* Copy one line - 80 bytes in ST high resolution */
- memcpy(pSTScreen, pVideoRaster, SCREENBYTES_MONOLINE);
+ if (offram)
+ memset(pSTScreen, 0, SCREENBYTES_MONOLINE);
+ else
+ memcpy(pSTScreen, pVideoRaster, SCREENBYTES_MONOLINE);
pVideoRaster += SCREENBYTES_MONOLINE;
/* Handle STE fine scrolling (HWScrollCount is zero on ST). */
if (HWScrollCount)
{
- Uint16 *pScrollAdj;
+ Uint16 *pScrollAdj, last16;
int nNegScrollCnt;
pScrollAdj = (Uint16 *)pSTScreen;
@@ -3438,8 +3437,9 @@ static void Video_CopyScreenLineMono(void)
}
/* Handle the last 16 pixels of the line */
+ last16 = offram ? 0 : do_get_mem_word(pVideoRaster);
do_put_mem_word(pScrollAdj, (do_get_mem_word(pScrollAdj) << HWScrollCount)
- | (do_get_mem_word(pVideoRaster) >> nNegScrollCnt));
+ | (last16 >> nNegScrollCnt));
/* HW scrolling advances Shifter video counter by one */
pVideoRaster += 1 * 2;
@@ -3598,8 +3598,7 @@ static void Video_CopyScreenLineColor(void)
if ((nHBL < nStartHBL) || (nHBL >= nEndHBL + BlankLines)
|| (LineBorderMask & ( BORDERMASK_EMPTY_LINE|BORDERMASK_NO_DE ) )
|| ShifterFrame.VBlank_signal
- || ( VerticalOverscan & V_OVERSCAN_NO_DE )
- || pVideoRaster + SCREENBYTES_MIDDLE > &STRam[STRamEnd] )
+ || ( VerticalOverscan & V_OVERSCAN_NO_DE ) )
{
/* Clear line to color '0' */
memset(pSTScreen, 0, SCREENBYTES_LINE);
@@ -3610,27 +3609,46 @@ static void Video_CopyScreenLineColor(void)
if ( LineBorderMask & ( BORDERMASK_LEFT_OFF | BORDERMASK_LEFT_OFF_MED ) ) /* bigger line by 26 bytes on the left */
{
pVideoRaster += BORDERBYTES_LEFT-SCREENBYTES_LEFT+VideoOffset;
- memcpy(pSTScreen, pVideoRaster, SCREENBYTES_LEFT);
+ if (OUTSIDE_RAM(pVideoRaster + SCREENBYTES_LEFT))
+ memset(pSTScreen, 0, SCREENBYTES_LEFT);
+ else
+ memcpy(pSTScreen, pVideoRaster, SCREENBYTES_LEFT);
pVideoRaster += SCREENBYTES_LEFT;
}
else if ( LineBorderMask & BORDERMASK_LEFT_OFF_2_STE ) /* bigger line by 20 bytes on the left (STE specific) */
{ /* bytes 0-3 are not shown, only next 16 bytes (32 pixels, 4 bitplanes) */
if ( SCREENBYTES_LEFT > BORDERBYTES_LEFT_2_STE )
{
- memset ( pSTScreen, 0, SCREENBYTES_LEFT-BORDERBYTES_LEFT_2_STE+4 ); /* clear unused pixels + bytes 0-3 */
- memcpy ( pSTScreen+SCREENBYTES_LEFT-BORDERBYTES_LEFT_2_STE+4, pVideoRaster+VideoOffset+4, BORDERBYTES_LEFT_2_STE-4 );
+ const int count = BORDERBYTES_LEFT_2_STE - 4;
+ int offset = SCREENBYTES_LEFT - BORDERBYTES_LEFT_2_STE + 4;
+ memset (pSTScreen, 0, offset); /* clear unused pixels + bytes 0-3 */
+ if (OUTSIDE_RAM(pVideoRaster + VideoOffset+4 + count))
+ memset(pSTScreen + offset, 0, count);
+ else
+ memcpy(pSTScreen + offset, pVideoRaster+VideoOffset+4, count);
}
else
- memcpy ( pSTScreen, pVideoRaster+BORDERBYTES_LEFT_2_STE-SCREENBYTES_LEFT+VideoOffset, SCREENBYTES_LEFT );
-
+ {
+ const int count = SCREENBYTES_LEFT;
+ int offset = BORDERBYTES_LEFT_2_STE-SCREENBYTES_LEFT+VideoOffset;
+ if (OUTSIDE_RAM(pVideoRaster + offset + count))
+ memset(pSTScreen, 0, SCREENBYTES_LEFT);
+ else
+ memcpy(pSTScreen, pVideoRaster + offset, count);
+ }
pVideoRaster += BORDERBYTES_LEFT_2_STE+VideoOffset;
}
else if (LineBorderMask & BORDERMASK_LEFT_PLUS_2) /* bigger line by 2 bytes on the left */
{
if ( SCREENBYTES_LEFT > 2 )
{
- memset(pSTScreen,0,SCREENBYTES_LEFT-2); /* clear unused pixels */
- memcpy(pSTScreen+SCREENBYTES_LEFT-2, pVideoRaster, 2);
+ const int count = 2;
+ int offset = SCREENBYTES_LEFT - count;
+ memset(pSTScreen, 0, offset); /* clear unused pixels */
+ if (OUTSIDE_RAM(pVideoRaster + count))
+ memset(pSTScreen+offset, 0, count);
+ else
+ memcpy(pSTScreen+offset, pVideoRaster, count);
}
else
{ /* nothing to copy, left border is not large enough */
@@ -3642,8 +3660,13 @@ static void Video_CopyScreenLineColor(void)
{
if ( SCREENBYTES_LEFT > 4*2 )
{
- memset(pSTScreen,0,SCREENBYTES_LEFT-4*2); /* clear unused pixels */
- memcpy(pSTScreen+SCREENBYTES_LEFT-4*2, pVideoRaster, 4*2);
+ const int count = 4*2;
+ int offset = SCREENBYTES_LEFT - count;
+ memset(pSTScreen, 0, offset); /* clear unused pixels */
+ if (OUTSIDE_RAM(pVideoRaster + count))
+ memset(pSTScreen+offset, 0, count);
+ else
+ memcpy(pSTScreen+offset, pVideoRaster, count);
}
else
{ /* nothing to copy, left border is not large enough */
@@ -3658,9 +3681,18 @@ static void Video_CopyScreenLineColor(void)
if (LineBorderMask & BORDERMASK_STOP_MIDDLE)
{
/* 106 bytes less in the line */
- memcpy(pSTScreen+SCREENBYTES_LEFT, pVideoRaster, SCREENBYTES_MIDDLE-106);
- memset(pSTScreen+SCREENBYTES_LEFT+SCREENBYTES_MIDDLE-106, 0, 106); /* clear unused pixels */
- pVideoRaster += (SCREENBYTES_MIDDLE-106);
+ const int count = SCREENBYTES_MIDDLE - 106;
+ if (OUTSIDE_RAM(pVideoRaster + count))
+ memset(pSTScreen+SCREENBYTES_LEFT, 0, count);
+ else
+ memcpy(pSTScreen+SCREENBYTES_LEFT, pVideoRaster, count);
+ memset(pSTScreen+SCREENBYTES_LEFT+count, 0, 106); /* clear unused pixels */
+ pVideoRaster += count;
+ }
+ else if (OUTSIDE_RAM(pVideoRaster + SCREENBYTES_MIDDLE))
+ {
+ memset(pSTScreen+SCREENBYTES_LEFT, 0, SCREENBYTES_MIDDLE);
+ pVideoRaster += SCREENBYTES_MIDDLE;
}
else
{
@@ -3680,7 +3712,10 @@ static void Video_CopyScreenLineColor(void)
/* Does have right border ? */
if (LineBorderMask & BORDERMASK_RIGHT_OFF)
{
- memcpy(pSTScreen+SCREENBYTES_LEFT+SCREENBYTES_MIDDLE, pVideoRaster, SCREENBYTES_RIGHT);
+ if (OUTSIDE_RAM(pVideoRaster + SCREENBYTES_RIGHT))
+ memset(pSTScreen+SCREENBYTES_LEFT+SCREENBYTES_MIDDLE, 0, SCREENBYTES_RIGHT);
+ else
+ memcpy(pSTScreen+SCREENBYTES_LEFT+SCREENBYTES_MIDDLE, pVideoRaster, SCREENBYTES_RIGHT);
pVideoRasterEndLine = pVideoRaster + SCREENBYTES_RIGHT;
pVideoRaster += BORDERBYTES_RIGHT;
}