Hi Larry, great, thanks for taking care of this so quickly! Having this in master is OK for us, the patches are simple enough for me to backport them if necessary. I ran across this when testing my OIIO integration into Cycles and wasn’t sure at first if I was doing something wrong.
-Stefan > On 7. Jul 2017, at 20:59, Larry Gritz <[email protected]> wrote: > > Stefan, > > Got it all now. There were two unrelated bugs here. > > First, for TIFF files (which have no way of actually storing channel names), > we were relying on the TIFF tag EXTRASAMPLES to signal alpha channels, and > when writing TIFF files (such as the default for maketx), we were neglecting > to set that tag at all when the number of channels was less than 4, > forgetting that you might have a lumanance+alpha 2-channel file. > > https://github.com/OpenImageIO/oiio/pull/1718 > <https://github.com/OpenImageIO/oiio/pull/1718> > > A totally unrelated error was plaguing your use of exr as a texture format, > which was that the "Y,A" channels were suffering from OpenEXR's frustrating > tendency to be obligated to shuffle channels into alphabetical order ("A" > comes before "Y"!). OIIO tries to reshuffle all the standard channels into > standard order (i.e. RGBA, not ABGR), but it wasn't considering "Y" in this > special case, even though our convention is to name 2-channel luma/alpha > images "Y,A". So now this convention is accounted for. > > https://github.com/OpenImageIO/oiio/pull/1717 > <https://github.com/OpenImageIO/oiio/pull/1717> > > > >> On Jul 7, 2017, at 12:28 AM, Larry Gritz <[email protected] >> <mailto:[email protected]>> wrote: >> >> OH, I get it now. My bad, you probably said that and it slipped my mind. >> >> Stay tuned. >> >> >>> On Jul 7, 2017, at 12:22 AM, Stefan Werner <[email protected] >>> <mailto:[email protected]>> wrote: >>> >>> Hi Larry, >>> >>> there’s a slight misunderstanding here. testtex using the png file works >>> fine for me too. What isn’t working for me is when I insert a conversion to >>> a mip map in between: >>> >>> so either: >>> $ dist/macosx/bin/maketx picture_frame_friedrich_nietzsche.png >>> >>> $ build/macosx/src/testtex/testtex -res 256 256 >>> picture_frame_friedrich_nietzsche.tx -graytorgb -o out.exr >>> >>> >>> >>> or >>> >>> $ dist/macosx/bin/maketx picture_frame_friedrich_nietzsche.png -o >>> picture_frame_friedrich_nietzsche.exr >>> >>> $build/macosx/src/testtex/testtex -res 256 256 >>> picture_frame_friedrich_nietzsche.exr -graytorgb -o out.exr >>> >>> give me screwed up colors. This is using OIIO master, commit >>> a4c784575b8e93e788f2e7fafd8a39fde68f364f (June 30th) >>> >>> My apologies, I realise now that I should have used the maketx/testtex >>> sequcence from the beginning, this makes it much easier to report issues >>> than trying to describe it it in words. >>> >>> -Stefan >>> >>> >>> >>>> On 6. Jul 2017, at 18:08, Larry Gritz <[email protected] >>>> <mailto:[email protected]>> wrote: >>>> >>>> Stefan, thanks for sending me (offline) the test case. >>>> >>>> Unfortunately, it works for me! >>>> >>>> Simple test: >>>> >>>> $ build/macosx/src/testtex/testtex -res 256 256 >>>> picture_frame_friedrich_nietzsche.png -o out.exr >>>> >>>> And I can see the colors are screwed up because channel 0 is being used >>>> for red, channel 1 (actually alpha) for green, and fill color for blue. >>>> >>>> Turn on the gray_to_rgb attribute: >>>> >>>> $ build/macosx/src/testtex/testtex -res 256 256 >>>> picture_frame_friedrich_nietzsche.png -graytorgb -o out.exr >>>> >>>> And it all looks fine. So I think it's working properly. Maybe whatever is >>>> going wrong is somehow specific to the sequence in which you are setting >>>> up the texture system, turning on the gray_to_rgb attribute, etc.? >>>> >>>> I tested both the current master as well as the current release branch >>>> (1.7.15). What version of OIIO are you using? Maybe you have stumbled >>>> across a problem that has already been fixed? >>>> >>>> Can you start by replicating the commands I have above and see if just the >>>> 'testtex' example works for you? >>>> >>>> >>>>> On Jul 5, 2017, at 4:59 AM, Stefan Werner <[email protected] >>>>> <mailto:[email protected]>> wrote: >>>>> >>>>> Reading the PNG appears to be correct, it gets identified as YA and >>>>> spec.alpha_channel is 1. I'm more concerned about the resulting .tx >>>>> (TIFF) or .exr files. iinfo also reports it as YA. >>>>> >>>>> When writing to .tx file, I can trace maketx through TIFFOutput::open() >>>>> and see that it writes a file with TIFFTAG_SAMPLESPERPIXEL=2. It does >>>>> however not set TIFFTAG_EXTRASAMPLES, as that only gets set when >>>>> m_spec.nchannels > 3. Later, when reading that .tx file, >>>>> TIFFInout::readspec() looks at TIFFTAG_EXTRASAMPLES to figure out whether >>>>> or not there is an alpha channel. As the tag is not present in the .tx >>>>> file, alpha_channel now remains -1 and the texture cache interprets it as >>>>> an RG TIFF file and does not recognise it as YA. iinfo also lists the >>>>> channels of the image as RG. >>>>> >>>>> For what it's worth, I notice that 3rd party programs have trouble >>>>> opening the resulting .tx file as TIFF. I suppose two channel TIFF files >>>>> are just not common enough? >>>>> >>>>> OpenEXR seems somewhat better. There the converted channels are marked as >>>>> Y and A, but the order is revesed - A has index 0, Y is at index 1. >>>>> However, fill_gray_channels() doesn't look for A or Y channels by their >>>>> names, it simply looks at whether an image has two channels with the >>>>> alpha being at index 1. The case of two channels, alpha at 0 seems to be >>>>> ignored and does not get converted from AY to YYYA. >>>>> >>>>> -Stefan >>>>> >>>>> On Tue, Jul 4, 2017 at 10:28 PM, Larry Gritz <[email protected] >>>>> <mailto:[email protected]>> wrote: >>>>> The idea is supposed to be that gray_to_rgb is only concerned with the >>>>> special case of turning Y->RGB and YA->RGBA (I'm using Y to signify >>>>> greyscale luminance). If it's a 2-channel image where the second channel >>>>> isn't alpha (let's call it JK), in general you don't want to wreck it by >>>>> changing it to a 4-channel image JJJK, that would be confusing. >>>>> >>>>> So it seems like the real bug might be that when the original image is >>>>> read, it's not setting alpha_channel correctly, so it's seeing a >>>>> 2-channel image but not recognizing it as the YA special case. >>>>> >>>>> In png_pvt.h, there is code circa line 7: >>>>> >>>>> if (spec.nchannels == 2) { >>>>> // Special case: PNG spec says 2-channel image is Gray & Alpha >>>>> spec.channelnames[0] = "Y"; >>>>> spec.channelnames[1] = "A"; >>>>> spec.alpha_channel = 1; >>>>> } >>>>> >>>>> that sure makes it seem like any 2-channel PNG image that is input will >>>>> necessarily know that channel 1 in alpha, so I'm not sure where things go >>>>> wrong. >>>>> >>>>> Can you maybe send me an image that exhibits this problem? I'd like to >>>>> trace through the logic and see how and why in the PNG gets confused >>>>> about what the channels mean. (Or, feel free to take a stab at that >>>>> yourself as well.) >>>>> >>>>> >>>>> > On Jul 4, 2017, at 12:40 PM, Stefan Werner <[email protected] >>>>> > <mailto:[email protected]>> wrote: >>>>> > >>>>> > Hi, >>>>> > >>>>> > I ran across one texture that I can’t quite figure out how to deal with >>>>> > it correctly using TextureSystem. The original texture is a gray scale >>>>> > PNG with alpha channel, and I’m converting it with maketx. I’m trying >>>>> > to get that back as RGBA via the gray_to_rgb=1 option in the texture >>>>> > cache, but I always get the gray channel as R and the alpha channel as >>>>> > G, with B and A empty/default. >>>>> > >>>>> > I stepped through the code, and I see that in >>>>> > TextureSystemImpl::fill_gray_channels(), line 848, the case for two >>>>> > channel input files only does the conversion when spec.alpha_channel = >>>>> > 1. When I use TIFF for my cache file format, spec.alpha_channel is -1, >>>>> > when I use OpenEXR as cache file format, spec.alpha_channel is 0. >>>>> > Maketx apparently only marks TIFF as containing an alpha channel when >>>>> > there are 4 or more channels, see tiffoutput.cpp line 485. OpenEXR does >>>>> > set the channel name to alpha. >>>>> > >>>>> > Is there a reason that fill_gray_channels() checks for index of the >>>>> > alpha channel rather than just its presence? Is there any way that I >>>>> > can get this texture to be read as RGBA through the texture cache? >>>>> > >>>>> > Thanks, >>>>> > Stefan >>>>> > _______________________________________________ >>>>> > Oiio-dev mailing list >>>>> > [email protected] <mailto:[email protected]> >>>>> > http://lists.openimageio.org/listinfo.cgi/oiio-dev-openimageio.org >>>>> > <http://lists.openimageio.org/listinfo.cgi/oiio-dev-openimageio.org> >>>>> >>>>> -- >>>>> Larry Gritz >>>>> [email protected] <mailto:[email protected]> >>>>> >>>>> >>>>> _______________________________________________ >>>>> Oiio-dev mailing list >>>>> [email protected] <mailto:[email protected]> >>>>> http://lists.openimageio.org/listinfo.cgi/oiio-dev-openimageio.org >>>>> <http://lists.openimageio.org/listinfo.cgi/oiio-dev-openimageio.org> >>>>> >>>>> _______________________________________________ >>>>> Oiio-dev mailing list >>>>> [email protected] <mailto:[email protected]> >>>>> http://lists.openimageio.org/listinfo.cgi/oiio-dev-openimageio.org >>>>> <http://lists.openimageio.org/listinfo.cgi/oiio-dev-openimageio.org> >>>> >>>> -- >>>> Larry Gritz >>>> [email protected] <mailto:[email protected]> >>>> >>>> >>>> _______________________________________________ >>>> Oiio-dev mailing list >>>> [email protected] <mailto:[email protected]> >>>> http://lists.openimageio.org/listinfo.cgi/oiio-dev-openimageio.org >>>> <http://lists.openimageio.org/listinfo.cgi/oiio-dev-openimageio.org> >>> >>> _______________________________________________ >>> Oiio-dev mailing list >>> [email protected] <mailto:[email protected]> >>> http://lists.openimageio.org/listinfo.cgi/oiio-dev-openimageio.org >>> <http://lists.openimageio.org/listinfo.cgi/oiio-dev-openimageio.org> >> >> -- >> Larry Gritz >> [email protected] <mailto:[email protected]> >> >> >> _______________________________________________ >> Oiio-dev mailing list >> [email protected] <mailto:[email protected]> >> http://lists.openimageio.org/listinfo.cgi/oiio-dev-openimageio.org > > -- > Larry Gritz > [email protected] <mailto:[email protected]> > > > _______________________________________________ > Oiio-dev mailing list > [email protected] > http://lists.openimageio.org/listinfo.cgi/oiio-dev-openimageio.org
_______________________________________________ Oiio-dev mailing list [email protected] http://lists.openimageio.org/listinfo.cgi/oiio-dev-openimageio.org
