[PATCH 2/9] windowscodecs: Add support for palette image formats to TIFF encoder.

Dmitry Timoshkov dmitry at baikal.ru
Wed Dec 5 14:40:57 CST 2018


Alexandre Julliard <julliard at winehq.org> wrote:

> >> > @@ -752,13 +752,13 @@ static void check_tiff_format(IStream *stream, const WICPixelFormatGUID *format)
> >> >      }
> >> >      else if (IsEqualGUID(format, &GUID_WICPixelFormat2bppIndexed))
> >> >      {
> >> > -        ok(width == 32, "wrong width %u\n", width);
> >> > +        ok(width == 16, "wrong width %u\n", width);
> >> >          ok(height == 2, "wrong height %u\n", height);
> >> >  
> >> > -        ok(bps == 1, "wrong bps %d\n", bps);
> >> > +        ok(bps == 2, "wrong bps %d\n", bps);
> >> >          ok(photo == 3, "wrong photometric %d\n", photo);
> >> >          ok(samples == 1, "wrong samples %d\n", samples);
> >> > -        ok(colormap == 6, "wrong colormap %d\n", colormap);
> >> > +        ok(colormap == 12, "wrong colormap %d\n", colormap);
> >> 
> >> If the test succeeds both before and after this change, doesn't it mean
> >> that 2bpp is never actually tested? What's the point of this test?
> >
> > It means that the original test is broken, and I had to fix it after
> > adding actual support for it in Wine.
> 
> Well, the broken test is added in your 1/9 patch...

That's correct, and I didn't notice that until I've spotted it.

> > That also means that since 2bpp
> > is not supported under Windows I missed that the test is broken while
> > running the test under Windows.
> 
> Which is my point, if 2bpp is not supported on Windows what's the point
> of supporting it in Wine? And having a test for it?
> 
> It seems to me that the correct test would be to verify that 2bpp
> doesn't work, and then Wine should conform to that.

The thing is that I first added support for palette formats in Wine, and
only after that added the tests and discovered that 2bpp format is not
supported under Windows. Then I just decided to leave the implementation
and mark the tests as broken. The reasoning that since there is an official
GUID for 2bpp format then nothing prevents it to be implemented in the codec:
even if Microsoft codecs don't support it other codecs may have this format
supported.

-- 
Dmitry.



More information about the wine-devel mailing list