[webkit-reviews] review granted: [Bug 244564] TextureMapper: Calculate zFar and zNear for the projection matrix : [Attachment 462019] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Aug 31 10:56:47 PDT 2022

Darin Adler <darin at apple.com> has granted Fujii Hironori
<Hironori.Fujii at sony.com>'s request for review:
Bug 244564: TextureMapper: Calculate zFar and zNear for the projection matrix

Attachment 462019: Patch


--- Comment #2 from Darin Adler <darin at apple.com> ---
Comment on attachment 462019
  --> https://bugs.webkit.org/attachment.cgi?id=462019

View in context: https://bugs.webkit.org/attachment.cgi?id=462019&action=review

> Source/WebCore/platform/graphics/texmap/TextureMapper.h:86
> +    virtual void setDepthRange(float, float) { }

Need argument names here. It’s not obvious what the two floats are.

This class is mixing float with double. TransformationMatrix uses double, but
we also use FloatRect and FloatPoint. It’s not obvious to me what would be

> Source/WebCore/platform/graphics/texmap/TextureMapperGL.cpp:872
> +static inline TransformationMatrix createProjectionMatrix(const IntSize&
size, bool mirrored, float zNear, float zFar)

Here we are converting floats to doubles when we put the results of our
expression into a TransformationMatrix object. Maybe they should just be
doubles from the start? I am not sure this mix of doubles and floats in the
existing code is OK.

I’d like to see tests that cover this somehow in the future.

> Source/WebCore/platform/graphics/texmap/TextureMapperLayer.h:38
> +struct TextureMapperComputeTransformData;

I think this struct should be a member of the TextureMapperLayer class. It’s an
implementation detail. I suggest naming it

More information about the webkit-reviews mailing list