Closed AnastaZIuk closed 3 years ago
done
I will do exactly what I did before but with no caching nor wasting memory for all the layers
Don't use auto
this much! It becomes illegible, use auto
only in generic/template code when you actually need it and in case of very long type names (but even then prefer typedef
/using
instead). auto i = 0u
isnt a good place to use auto
for sure!
Actually... i dont know why you even touch output regions while decoding.. Just do CBasicImageFilterCommon::executePerRegion(commonExecuteData.inImg, decode, commonExecuteData.inRegions.begin(), commonExecuteData.inRegions.end(), clip);
, fill scratch with decoded texels, do SAT on scratch, and finally executePerRegion
over output regions (encode). Ofc all of the above should be done per layer. Take into account my today's review.
Yout negative offsets problem must be result of wrong API usage (of which is great amount as I already found quite a few in review comments), but theres no point in looking for whats wrong because it can be redesigned and done in much simpler way (the main part, (i.e. SAT code) most likely doesnt need any changes)
Don't use
auto
this much! It becomes illegible, useauto
only in generic/template code when you actually need it and in case of very long type names (but even then prefertypedef
/using
instead).auto i = 0u
isnt a good place to useauto
for sure!
Also auto
without a reference (auto&
) turns into a "copy-by-value" which has other performance implications.
OK I think clip_functor_t
needs an explainer
https://github.com/buildaworldnet/IrrlichtBAW/blob/shader_pipeline/include/irr/asset/filters/CBasicImageFilterCommon.h#L58
You create the object by giving it the subresource (mip level and layer) of the original image, as well as the offset + extent to clip to, these are pretty much derived from your Filter's State.
you also pass it some texel-block-format info, but thats just for convenience and speed (it can derive those from the format).
Whenever you call the clip functor's function operator
https://github.com/buildaworldnet/IrrlichtBAW/blob/shader_pipeline/include/irr/asset/filters/CBasicImageFilterCommon.h#L69
it will produce a newRegion
that is the intersection of the offset and extent of the functor and the offset and extent of referenceRegion
.
Now in order to produce such a region so its valid and is "as-if" it specified the image itself, you need to do the following:
bufferWidth
if it was 0/-1
and similar because you need to know the byte-distance between successive rows and layers in a clipped regionreferenceRegion
(maximum of the coordinates)targetLimit
is the the one-past the max coordinate of the rectangle given in the constructor, while resultLimit
is the equivalent for the input region.bufferOffset
given the change in the offset
(offset can only become larger than the reference region so the bufferOffset
shall be increased by a non-negative number)You can see step 2 and 3 being done starting on this line: https://github.com/buildaworldnet/IrrlichtBAW/blob/shader_pipeline/include/irr/asset/filters/CBasicImageFilterCommon.h#L82
If the two rectangles don't intersect (no common texels) then we return false
https://github.com/buildaworldnet/IrrlichtBAW/blob/shader_pipeline/include/irr/asset/filters/CBasicImageFilterCommon.h#L84
The offsetInOffset
cannot be negative:
https://github.com/buildaworldnet/IrrlichtBAW/blob/shader_pipeline/include/irr/asset/filters/CBasicImageFilterCommon.h#L90
Status:
almost done