[WebGL] Validation functions for compressed formats incorrect

• Component: WebGL
• Status: RESOLVED FIXED

[Brent Fulgham]

The calculation of valid size ranges for various compressed texture types is currently incorrect:

See http://www.khronos.org/registry/gles/extensions/IMG/IMG_texture_compression_pvrtc.txt.

1. COMPRESSED_RGB_PVRTC_2BPPV1_IMG and COMPRESSED_RGBA_PVRTC_2BPPV1_IMG

These are specified to be calculated as:
          ( max(width, 16) * max(height, 8) * 2 + 7) / 8
... but are currently calculated as:
          std::max(width, 8) * std::max(height, 8) / 4

2. COMPRESSED_RGB_PVRTC_4BPPV1_IMG and COMPRESSED_RGBA_PVRTC_4BPPV1_IMG:

These are specified to be calculated as:
          ( max(width, 8) * max(height, 8) * 4 + 7) / 8
... but are currently calculated as:
          std::max(width, 8) * std::max(height, 8) / 2

[Radar WebKit Bug Importer]

<rdar://problem/15820935>

[Brent Fulgham]

Created attachment 221233 [details]
Patch

[Brent Fulgham]

Comment on attachment 221233 [details]
Patch

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

> Source/WebCore/html/canvas/WebGLRenderingContext.cpp:5351
> +            bytesRequired = numBlocksAcross * numBlocksDown * kBlockSize;

This case is actually identical to the COMPRESSED_RGB_S3TC_DXT1_EXT/COMPRESSED_RGBA_S3TC_DXT1_EXT one.

> Source/WebCore/html/canvas/WebGLRenderingContext.cpp:5362
> +            bytesRequired = numBlocksAcross * numBlocksDown * kBlockSize;

And these are the same as COMPRESSED_RGBA_S3TC_DXT3_EXT/COMPRESSED_RGBA_S3TC_DXT5_EXT.

[Brent Fulgham]

The WebGL specification refers to 'floor' when defining some of these relationships (see http://www.khronos.org/registry/webgl/extensions/WEBGL_compressed_texture_s3tc/).

However, I performing integer divides (on positive values) will provide the same values without having to convert integers to doubles followed by a floor on the result of a divide. Since we check for negative arguments at the start of the function, this should be safe.

[Brent Fulgham]

Created attachment 221288 [details]
Simple Test of Size Calculations

Simple program to identify power-of-two dimensions where the old WebKit calculation does not match the specification.

[Brent Fulgham]

It looks like the old calculation was probably fine for almost all cases. I found a few where we would produce an invalid result previously:

fulgbr-pc:compression bfulgham$ ./comparison 
2BPPV1: (1, 1) Old: 16, Corrected: 32
2BPPV1: (1, 2) Old: 16, Corrected: 32
2BPPV1: (1, 4) Old: 16, Corrected: 32
2BPPV1: (1, 8) Old: 16, Corrected: 32
2BPPV1: (1, 16) Old: 32, Corrected: 64
2BPPV1: (1, 32) Old: 64, Corrected: 128
2BPPV1: (1, 64) Old: 128, Corrected: 256
2BPPV1: (1, 128) Old: 256, Corrected: 512
2BPPV1: (1, 256) Old: 512, Corrected: 1024
2BPPV1: (1, 512) Old: 1024, Corrected: 2048
2BPPV1: (1, 1024) Old: 2048, Corrected: 4096
2BPPV1: (1, 2048) Old: 4096, Corrected: 8192
2BPPV1: (1, 4096) Old: 8192, Corrected: 16384
2BPPV1: (1, 8192) Old: 16384, Corrected: 32768
2BPPV1: (2, 1) Old: 16, Corrected: 32
2BPPV1: (2, 2) Old: 16, Corrected: 32
2BPPV1: (2, 4) Old: 16, Corrected: 32
2BPPV1: (2, 8) Old: 16, Corrected: 32
2BPPV1: (2, 16) Old: 32, Corrected: 64
2BPPV1: (2, 32) Old: 64, Corrected: 128
2BPPV1: (2, 64) Old: 128, Corrected: 256
2BPPV1: (2, 128) Old: 256, Corrected: 512
2BPPV1: (2, 256) Old: 512, Corrected: 1024
2BPPV1: (2, 512) Old: 1024, Corrected: 2048
2BPPV1: (2, 1024) Old: 2048, Corrected: 4096
2BPPV1: (2, 2048) Old: 4096, Corrected: 8192
2BPPV1: (2, 4096) Old: 8192, Corrected: 16384
2BPPV1: (2, 8192) Old: 16384, Corrected: 32768
2BPPV1: (4, 1) Old: 16, Corrected: 32
2BPPV1: (4, 2) Old: 16, Corrected: 32
2BPPV1: (4, 4) Old: 16, Corrected: 32
2BPPV1: (4, 8) Old: 16, Corrected: 32
2BPPV1: (4, 16) Old: 32, Corrected: 64
2BPPV1: (4, 32) Old: 64, Corrected: 128
2BPPV1: (4, 64) Old: 128, Corrected: 256
2BPPV1: (4, 128) Old: 256, Corrected: 512
2BPPV1: (4, 256) Old: 512, Corrected: 1024
2BPPV1: (4, 512) Old: 1024, Corrected: 2048
2BPPV1: (4, 1024) Old: 2048, Corrected: 4096
2BPPV1: (4, 2048) Old: 4096, Corrected: 8192
2BPPV1: (4, 4096) Old: 8192, Corrected: 16384
2BPPV1: (4, 8192) Old: 16384, Corrected: 32768
2BPPV1: (8, 1) Old: 16, Corrected: 32
2BPPV1: (8, 2) Old: 16, Corrected: 32
2BPPV1: (8, 4) Old: 16, Corrected: 32
2BPPV1: (8, 8) Old: 16, Corrected: 32
2BPPV1: (8, 16) Old: 32, Corrected: 64
2BPPV1: (8, 32) Old: 64, Corrected: 128
2BPPV1: (8, 64) Old: 128, Corrected: 256
2BPPV1: (8, 128) Old: 256, Corrected: 512
2BPPV1: (8, 256) Old: 512, Corrected: 1024
2BPPV1: (8, 512) Old: 1024, Corrected: 2048
2BPPV1: (8, 1024) Old: 2048, Corrected: 4096
2BPPV1: (8, 2048) Old: 4096, Corrected: 8192
2BPPV1: (8, 4096) Old: 8192, Corrected: 16384
2BPPV1: (8, 8192) Old: 16384, Corrected: 32768

[Brent Fulgham]

Created attachment 221291 [details]
Patch

[Brent Fulgham]

Committed r162085: <http://trac.webkit.org/changeset/162085>