Within PostProcessTonemap.cpp, there exists both a TonemapperConfBitmaskPC array and a TonemapperConfBitmaskMobile array. The comment at the top says…
// Edit the following to add and remove configurations.
// This is a white list of the combinations which are compiled.
// Place most common first (faster when searching in TonemapperFindLeastExpensive()).
At the end of TonemapperFindLeastExpensive, if it fails to find a matching configuration then it return the first entry and has the following comment…
// Fail returns 0, the gamma only shader.
TonemapperConfBitmaskMobile actually has the gamma only shader as the first, with increasingly more complex shaders after.
TonemapperConfBitmaskPC is the exact opposite with the most complex first. So the gamma comment appears to be contradictory.
I mention all this because after upgrading to 4.8 I’m having to go through PostProcessTonemap and figure out why I am having a bunch of issues and the contradictory comments are not helping. Can this be fixed? Which is the correct pattern?
I believe I did discover that the reduced list of configurations in TonemapperConfBitmaskPC had resulted in some of my shaders now being compiled with the first (most complex) shader. Why was this list reduced?
HI JasonKng -
Version 4.8 included a massive addition and performance improvements for Post Processing, read more here, and in particular a new Film Standard Tonemapper was added. In reference to the code comments, whether the Gamma only shader is at the beginning or the end, the Least Expensive Code will default to that Table Entry. Least expensive function actively searches the table each time. The Most common configuration for PCs are the most expensive post processing setup, while for Mobile the most common is the simple Gamma Shader, which is what the initial comment addresses.
There have been some issues with the 4.8 post process settings and the new tonemapper settings, but they are unrelated to this particular bit of coding and we are currently working on making improvements and many have been implemented in 4.9.
Thank You -
Eric Ketchum
Hi Eric,
Thanks for your response!
I had a few uses of Bloom and Contrast (and other similar) that were suddenly using the default. I had to add back in a few configuration so that I was not using the default. Should that have been expected? It looks like Bloom is still a part of the various default configurations, but Contrast is not. If that’s been incorporated elsewhere then why is it showing up as an option or why was it removed as part of the configurations? I want to make sure that everything is running optimally so I want to make sure that adding these configurations back in was warranted.
Thanks,
The comments are confusing - seems the code has changes and the comments haven’t been updated.
It would be nice to have PC and mobile doing it the same order. You clearly identified some issues there.
Generally this code is meant to pick the fastest possible shader permutation - the most complex shader should cover
all the other cases.
I created “UE-19535 Tonemapper shader permutation code/comment are confusing and behaves wrong in corner cases”
to track the issue and get it resolved.
The reason many bits were removed from the PC list was that those features have been moved to a LUT outside of the tone mapping shader. We don’t need permutations to optimize these different features being on. This was a large simplification to the tone mapper. The number of permutations shrank dramatically. It also resulted in a healthy optimization. I’d like to eventually do the same with the mobile path but I need to make absolutely sure it is at least as fast as what is there currently.
The only bit that was removed from the mobile list was vignette color which is no longer supported. The feature was removed. It being removed should not affect whether a match is found since it will no longer be looking for that bit.
The fail case should never happen. There should be an entry in the list that has all possible features which it will find a costly match with. Maybe there should be a check() there instead if it doesn’t find anything. The order is done such that the first entry is the expected most common and the last is least common. This will help the exact match case. If there isn’t an exact match the order doesn’t matter.
I do see a bug for the PC path which adds bits in TonemapperGenerateBitmask for features which aren’t in the PC list anymore. I just added code to wipe out those bits in TonemapperGenerateBitmaskPC.