Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add unpackColorSpace tests to tex-image-and-sub-image-2d-with-video #3404

Merged
merged 5 commits into from
May 17, 2022
Merged

Add unpackColorSpace tests to tex-image-and-sub-image-2d-with-video #3404

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
4d39960
Add unpackColorSpace tests to tex-image-and-sub-image-2d-with-video
ccameron-chromium Apr 27, 2022
d2e0a53
Fix lint errors
ccameron-chromium Apr 27, 2022
7136180
Only test RGB, fix LUMINANCE alpha channel
ccameron-chromium May 4, 2022
faf3fdf
Incorporate review feedback
ccameron-chromium May 4, 2022
f77ed50
Incorporate review feedback
ccameron-chromium May 9, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 30 additions & 36 deletions sdk/tests/js/tests/tex-image-and-sub-image-2d-with-video.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,6 @@ function generateTest(internalFormat, pixelFormat, pixelType, prologue, resource
var tiu = TexImageUtils;
var gl = null;
var successfullyParsed = false;
var redColor = [255, 0, 0];
var greenColor = [0, 255, 0];

// Test each format separately because many browsers implement each
// differently. Some might be GPU accelerated, some might not. Etc...
Expand All @@ -45,31 +43,13 @@ function generateTest(internalFormat, pixelFormat, pixelType, prologue, resource
return;
}

switch (gl[pixelFormat]) {
case gl.RED:
case gl.RED_INTEGER:
greenColor = [0, 0, 0];
break;
case gl.LUMINANCE:
case gl.LUMINANCE_ALPHA:
redColor = [255, 255, 255];
greenColor = [0, 0, 0];
break;
case gl.ALPHA:
redColor = [0, 0, 0];
greenColor = [0, 0, 0];
break;
default:
break;
}

gl.clearColor(0,0,0,1);
gl.clearDepth(1);

runTest();
}

function runOneIteration(videoElement, useTexSubImage2D, flipY, topColor, bottomColor, sourceSubRectangle, program, bindingTarget)
function runOneIteration(videoElement, unpackColorSpace, useTexSubImage2D, flipY, topColorName, bottomColorName, sourceSubRectangle, program, bindingTarget)
{
sourceSubRectangleString = '';
if (sourceSubRectangle) {
Expand Down Expand Up @@ -102,6 +82,10 @@ function generateTest(internalFormat, pixelFormat, pixelType, prologue, resource
gl.TEXTURE_CUBE_MAP_POSITIVE_Z,
gl.TEXTURE_CUBE_MAP_NEGATIVE_Z];
}
// Handle target color space.
if (unpackColorSpace) {
gl.unpackColorSpace = unpackColorSpace;
kenrussell marked this conversation as resolved.
Show resolved Hide resolved
}
// Handle the source sub-rectangle if specified (WebGL 2.0 only)
if (sourceSubRectangle) {
gl.pixelStorei(gl.UNPACK_SKIP_PIXELS, sourceSubRectangle[0]);
Expand Down Expand Up @@ -165,6 +149,12 @@ function generateTest(internalFormat, pixelFormat, pixelType, prologue, resource
loc = gl.getUniformLocation(program, "face");
}

const topColor = wtu.colorAsSampledWithInternalFormat(
wtu.namedColorInColorSpace(topColorName, unpackColorSpace),
internalFormat);
const bottomColor = wtu.colorAsSampledWithInternalFormat(
wtu.namedColorInColorSpace(bottomColorName, unpackColorSpace),
internalFormat);
for (var tt = 0; tt < targets.length; ++tt) {
if (bindingTarget == gl.TEXTURE_CUBE_MAP) {
gl.uniform1i(loc, targets[tt]);
Expand All @@ -173,7 +163,7 @@ function generateTest(internalFormat, pixelFormat, pixelType, prologue, resource
wtu.clearAndDrawUnitQuad(gl, [0, 0, 0, 255]);
// Check a few pixels near the top and bottom and make sure they have
// the right color.
const tolerance = 6;
const tolerance = Math.max(6, tiu.tolerance(internalFormat, pixelFormat, pixelType));
debug("Checking lower left corner");
wtu.checkCanvasRect(gl, 4, 4, 2, 2, bottomColor,
"shouldBe " + bottomColor, tolerance);
Expand All @@ -186,33 +176,36 @@ function generateTest(internalFormat, pixelFormat, pixelType, prologue, resource
function runTest(videoElement)
{
var cases = [
{ sub: false, flipY: true, topColor: redColor, bottomColor: greenColor },
{ sub: false, flipY: false, topColor: greenColor, bottomColor: redColor },
{ sub: true, flipY: true, topColor: redColor, bottomColor: greenColor },
{ sub: true, flipY: false, topColor: greenColor, bottomColor: redColor },
{ sub: false, flipY: true, topColor: 'Red', bottomColor: 'Green' },
{ sub: false, flipY: false, topColor: 'Green', bottomColor: 'Red' },
{ sub: true, flipY: true, topColor: 'Red', bottomColor: 'Green' },
{ sub: true, flipY: false, topColor: 'Green', bottomColor: 'Red' },
];

if (wtu.getDefault3DContextVersion() > 1) {
cases = cases.concat([
{ sub: false, flipY: false, topColor: redColor, bottomColor: redColor,
{ sub: false, flipY: false, topColor: 'Red', bottomColor: 'Red',
sourceSubRectangle: [20, 16, 40, 32] },
{ sub: false, flipY: true, topColor: greenColor, bottomColor: greenColor,
{ sub: false, flipY: true, topColor: 'Green', bottomColor: 'Green',
sourceSubRectangle: [20, 16, 40, 32] },
{ sub: false, flipY: false, topColor: greenColor, bottomColor: greenColor,
{ sub: false, flipY: false, topColor: 'Green', bottomColor: 'Green',
sourceSubRectangle: [20, 80, 40, 32] },
{ sub: false, flipY: true, topColor: redColor, bottomColor: redColor,
{ sub: false, flipY: true, topColor: 'Red', bottomColor: 'Red',
sourceSubRectangle: [20, 80, 40, 32] },
{ sub: true, flipY: false, topColor: redColor, bottomColor: redColor,
{ sub: true, flipY: false, topColor: 'Red', bottomColor: 'Red',
sourceSubRectangle: [20, 16, 40, 32] },
{ sub: true, flipY: true, topColor: greenColor, bottomColor: greenColor,
{ sub: true, flipY: true, topColor: 'Green', bottomColor: 'Green',
sourceSubRectangle: [20, 16, 40, 32] },
{ sub: true, flipY: false, topColor: greenColor, bottomColor: greenColor,
{ sub: true, flipY: false, topColor: 'Green', bottomColor: 'Green',
sourceSubRectangle: [20, 80, 40, 32] },
{ sub: true, flipY: true, topColor: redColor, bottomColor: redColor,
{ sub: true, flipY: true, topColor: 'Red', bottomColor: 'Red',
sourceSubRectangle: [20, 80, 40, 32] },
]);
}

cases = tiu.crossProductTestCasesWithUnpackColorSpaces(
cases, tiu.unpackColorSpacesToTest(gl));

function runTexImageTest(bindingTarget) {
var program;
if (bindingTarget == gl.TEXTURE_2D) {
Expand Down Expand Up @@ -269,8 +262,9 @@ function generateTest(internalFormat, pixelFormat, pixelType, prologue, resource
break;
}
}
runOneIteration(video, cases[i].sub, cases[i].flipY,
cases[i].topColor, cases[i].bottomColor,
runOneIteration(video, cases[i].unpackColorSpace, cases[i].sub, cases[i].flipY,
cases[i].topColor,
cases[i].bottomColor,
cases[i].sourceSubRectangle,
program, bindingTarget);
}
Expand Down
62 changes: 61 additions & 1 deletion sdk/tests/js/tests/tex-image-and-sub-image-utils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -791,11 +791,71 @@ var TexImageUtils = (function() {
return program;
};

/**
* Return a list of unpack color spaces to test, supported by the specified
* WebGLRenderingContext.
*/
var unpackColorSpacesToTest = function(gl)
{
if ('unpackColorSpace' in gl)
return ['srgb', 'display-p3'];
else
return [null];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be better to use undefined rather than null here? (Maybe not.)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

changed to undefined.

}

/**
* For each entry in unpackColorSpaces, duplicate all of cases, adding an
* unpackColorSpace key with its value set to that entry to each case.
*/
var crossProductTestCasesWithUnpackColorSpaces = function(cases, unpackColorSpaces)
{
var caseWithColorSpace = function(c, cs) { return Object.assign({}, c, {unpackColorSpace:cs}); }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

return Object.assign({}, c, {unpackColorSpace:cs})

can be shortened to

return {...c, ...{unpackColorSpace:cs}};

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This didn't work when I tested against FF, so I left it unchanged.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How did you test? Works in Firefox just fine AFAIK

https://jsgist.org/?src=c212a3fa63ff82bc55c88b73fd996b1b

I've been using it for years

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, no idea what I did wrong. Now it works fine. Updated.

var casesList = unpackColorSpaces.map(cs => cases.map(c => caseWithColorSpace(c, cs)));
return [].concat.apply([], casesList);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could this just be

return caseList;

?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

casesList ends up being
[[list of cases with srgb], [list of cases with display-p3]]
and so the concat moves them all into a single list.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In that case you can use flat

return caseList.flat()

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated.

}

/**
* Given given an internalformat, format, and type, return the tolerance
* that should be used when comparing an input 8-bit value to one that has
* been truncated through the specified formats.
*/
var tolerance = function(internalformat, format, type) {
function typeTolerance(type) {
switch(type) {
case 'UNSIGNED_SHORT_5_6_5':
case 'UNSIGNED_SHORT_5_5_5_1':
return 255 / 31;
case 'UNSIGNED_SHORT_4_4_4_4':
return 255 / 15;
break;
default:
return 1;
}
};
function formatTolerance(format) {
switch(format) {
case 'RGB565':
case 'RGB5_A1':
return 255/31;
case 'RGBA4':
return 255/15;
default:
return 1;
}
};
return Math.max(formatTolerance(internalformat),
formatTolerance(format),
typeTolerance(type));
}

return {
setupTexturedQuad: setupTexturedQuad,
setupTexturedQuadWithCubeMap: setupTexturedQuadWithCubeMap,
setupTexturedQuadWith3D: setupTexturedQuadWith3D,
setupTexturedQuadWith2DArray: setupTexturedQuadWith2DArray
setupTexturedQuadWith2DArray: setupTexturedQuadWith2DArray,
unpackColorSpacesToTest: unpackColorSpacesToTest,
crossProductTestCasesWithUnpackColorSpaces: crossProductTestCasesWithUnpackColorSpaces,
tolerance: tolerance
};

}());
92 changes: 92 additions & 0 deletions sdk/tests/js/webgl-test-utils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3296,6 +3296,94 @@ function linearChannelToSRGB(value) {
return Math.trunc(value * 255 + 0.5);
}

/**
* Return the named color in the specified color space.
* @param {string} colorName The name of the color to convert.
* Supported color names are:
* 'Red', which is maximum sRGB red.
* 'Green', which is maximum sRGB green.
* @param {string} colorSpace The color space to convert to. Supported
color spaces are:
* null, which is treated as sRGB
* 'srgb'
* 'display-p3'.
* @return {!Array.<number>} color The color in the specified color
* space as an 8-bit RGBA array with unpremultiplied alpha.
*/
var namedColorInColorSpace = function(colorName, colorSpace) {
var result;
switch (colorSpace) {
case null:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If null is changed to undefined in unpackColorSpacesToTest, above, then this would have to be changed too.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

case 'srgb':
switch(colorName) {
case 'Red':
return [255, 0, 0, 255];
case 'Green':
return [0, 255, 0, 255];
break;
default:
throw 'unexpected color name: ' + colorName;
};
break;
case 'display-p3':
switch(colorName) {
case 'Red':
return [234, 51, 35, 255];
kenrussell marked this conversation as resolved.
Show resolved Hide resolved
break;
case 'Green':
return [117, 251, 76, 255];
kenrussell marked this conversation as resolved.
Show resolved Hide resolved
break;
default:
throw 'unexpected color name: ' + colorName;
}
break;
default:
throw 'unexpected color space: ' + colorSpace;
}
}

/**
* Return the named color as it would be sampled with the specified
* internal format
* @param {!Array.<number>} color The color as an 8-bit RGBA array.
* @param {string} internalformat The internal format.
* @return {!Array.<number>} color The color, as it would be sampled by
* the specified internal format, as an 8-bit RGBA array.
*/
var colorAsSampledWithInternalFormat = function(color, internalFormat) {
switch (internalFormat) {
case 'ALPHA':
return [0, 0, 0, color[3]];
case 'LUMINANCE':
case 'LUMINANCE_ALPHA':
return [color[0], color[0], color[0], color[3]];
case 'SRGB8':
case 'SRGB8_ALPHA8':
return [sRGBChannelToLinear(color[0]),
sRGBChannelToLinear(color[1]),
sRGBChannelToLinear(color[2]),
color[3]];
case 'R16F':
case 'R32F':
case 'R8':
case 'R8UI':
case 'RED':
case 'RED_INTEGER':
return [result[0], 0, 0, 0];
case 'RG':
case 'RG16F':
case 'RG32F':
case 'RG8':
case 'RG8UI':
case 'RG_INTEGER':
return [color[0], color[1], 0, 0];
break;
default:
break;
}
return color;
}

function comparePixels(cmp, ref, tolerance, diff) {
if (cmp.length != ref.length) {
testFailed("invalid pixel size.");
Expand Down Expand Up @@ -3516,6 +3604,10 @@ var API = {
// fullscreen api
setupFullscreen: setupFullscreen,

// color converter API
namedColorInColorSpace: namedColorInColorSpace,
colorAsSampledWithInternalFormat: colorAsSampledWithInternalFormat,

// sRGB converter api
sRGBToLinear: sRGBToLinear,
linearToSRGB: linearToSRGB,
Expand Down