Re: [hatari-devel] Less dark doubled TV-monitor mode for 32-bit output

[Thomas Huth <th.huth@xxxxxxxxx>]

Am Wed, 6 Nov 2019 01:31:32 +0200
schrieb Eero Tamminen <oak@xxxxxxxxxxxxxx>:

> Hi Thomas,
> 
> On 11/5/19 2:00 AM, Eero Tamminen wrote:
> > On 11/4/19 11:51 AM, Thomas Huth wrote:  
> ...
> >> This code is likely not endianess safe:  
> ...
> >> You have to use the values from SDL_PixelFormat if you want to
> >> make it run properly everywhere.  
> > 
> > With all the 8-bit channels getting the same treatment, it doesn't
> > matter in which order they are.  See attached updated patch.  
> 
> Note that screen.c requests specific 32-bit and 16-bit screen types
> in Screen_SetSDLVideoSize():
> -----------------------------------------------------
>                  if (bitdepth == 16)
>                  {
>                          rm = 0xF800;
>                          gm = 0x07E0;
>                          bm = 0x001F;
>                          pfmt = SDL_PIXELFORMAT_RGB565;
>                  }
>                  else
>                  {
>                          rm = 0x00FF0000;
>                          gm = 0x0000FF00;
>                          bm = 0x000000FF;
>                          pfmt = SDL_PIXELFORMAT_RGB888;
>                  }

That's the SDL2 part only. Not sure, but I think this might be
sub-optimal on certain byte-swapped screen surfaces... not sure
whether SDL2 supports a way to query the native format, though, but
if there is a way, we should change that code.

SDL1 takes the best available format already, IIRC.

> and ST screen conversion macros in src/convert/macros.h rely
> on them, they don't use SDL_PixelFormat.

No, they don't rely on these hard-coded values. They use STRGBPalette[]
which should contain the right values.

> It seems that only AVI recording and screenshot functionality
> use SDL_PixelFormat (through PixelConvert_* functions).

We've had endianess issues with the AVI code in the past, that's why it
uses PixelFormat. It's needed. Really. Really really.

So instead of discussing, why don't you simply try it?

Something like this (not tested, just scribbled here):

    uint32_t *next, *line;
    SDL_PixelFormat *fmt = surf->format;
    uint32_t mask = (fmt->Rmask >> 1) & fmt->Rmask)
                    | (fmt->Gmask >> 1) & fmt->Gmask)
                    | (fmt->Bmask >> 1) & fmt->Bmask);

    ... loop ...
    {
          *next++ = (*line++ >> 1) & mask;
    }

That's certainly less code in the loop body than shifting each byte on
its own.

 Thomas