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

Jump to bottom

Puppeteer E2E test: Regenerate high-differenсe screenshots and decrease threshold #25403

Merged
merged 4 commits into from Feb 6, 2023
Merged

Puppeteer E2E test: Regenerate high-differenсe screenshots and decrease threshold #25403

merged 4 commits into from Feb 6, 2023

Conversation

LeviPesin
Copy link
Contributor

Related issue: #24109

Description

Regenerate screenshots that produce at least 0.3% difference on at least one platform:

css3d_periodictable
misc_boxselection
misc_controls_pointerlock
misc_uv_tests
webgl2_multisampled_renderbuffers
webgl2_ubo
webgl_animation_skinning_blending
webgl_buffergeometry_compression
webgl_buffergeometry_drawrange
webgl_camera
webgl_camera_logarithmicdepthbuffer
webgl_clipping_advanced
webgl_custom_attributes_lines
webgl_custom_attributes_points2
webgl_custom_attributes_points3
webgl_framebuffer_texture
webgl_geometry_extrude_splines
webgl_gpgpu_birds_gltf
webgl_gpgpu_protoplanet
webgl_helpers
webgl_interactive_raycasting_points
webgl_lightningstrike
webgl_lights_pointlights
webgl_lights_spotlights
webgl_loader_collada_kinematics
webgl_loader_gcode
webgl_loader_gltf_transmission
webgl_loader_ldraw
webgl_loader_md2
webgl_loader_mmd_audio
webgl_loader_mmd_pose
webgl_loader_pcd
webgl_loader_pdb
webgl_loader_svg
webgl_loader_tilt
webgl_lod
webgl_marchingcubes
webgl_materials_blending
webgl_materials_blending_custom
webgl_materials_variations_lambert
webgl_materials_variations_phong
webgl_materials_variations_physical
webgl_materials_variations_standard
webgl_materials_variations_toon
webgl_materials_wireframe
webgl_multiple_canvases_circle
webgl_multiple_elements_text
webgl_nodes_loader_materialx
webgl_nodes_materials_instance_uniform
webgl_nodes_playground
webgl_points_dynamic
webgl_postprocessing_fxaa
webgl_postprocessing_pixel
webgl_postprocessing_ssr
webgl_postprocessing_advanced
webgl_raycaster_bvh
webgl_shaders_tonemapping
webgl_shadowmap_performance
webgpu_nodes_playground
webxr_vr_layers
webxr_vr_video
webxr_vr_sandbox

Generated using Chromium 109.0.5414.120 (revision 1070054, https://storage.googleapis.com/chromium-browser-snapshots/Win_x64/1070054/chrome-win.zip), stable channel on win64.

@LeviPesin LeviPesin marked this pull request as draft February 2, 2023 07:39
@LeviPesin
Copy link
Contributor Author

LeviPesin commented Feb 2, 2023 •
edited

Not sure what is the noise in webgl_camera...
Some screenshots (css3d_periodictable, misc_controls_pointerlock, misc_uv_tests, webgl_camera_logarithmicdepthbuffer, webgl_loader_ldraw, webgl_loader_pdb, webgl_multiple_canvases_circle, webgl_multiple_elements_text, webgl_shaders_tonemapping, webgpu_nodes_playground) only differ in the text displayed, this is strange -- it should be cleaned by the clean_page.js... Maybe we should somehow set a default font in Puppeteer so there would not be such Windows/Linux/Mac differences in text rendering?
webgl_loader_md2 and webgl_loader_mmd_audio are fixed now!
Most changes are due to Linear->sRGB change in some loaders (or some local changes in examples that were forgotten to be re-screenshoted -- e.g. webgl_nodes_playground or webgl_postprocessing_fxaa), I think.

Remaining high differences (most are due to different text font on Windows vs Linux):

css3d_periodictable
misc_controls_pointerlock
misc_uv_tests
webgl_clipping_advanced
webgl_lights_spotlights
webgl_loader_pdb
webgl_materials_blending
webgl_multiple_canvases_circle
webgl_multiple_elements_text
webgl_nodes_playground
webgl_shaders_tonemapping
webgpu_nodes_playground
webxr_vr_video

@LeviPesin LeviPesin changed the title Puppeteer E2E test: Regenerate high-differene screenshots Puppeteer E2E test: Regenerate high-differenсe screenshots Feb 2, 2023
LeviPesin
LeviPesin commented Feb 2, 2023
View reviewed changes
test/e2e/puppeteer.js Outdated
'webxr_ar_lighting', // webxr

'webgl_worker_offscreencanvas', // in a worker, not robust

// Windows-Linux text rendering differences
// TODO: Fix these by setting a font in Puppeteer
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will try to fix these in a separate PR.

LeviPesin
LeviPesin commented Feb 2, 2023
View reviewed changes
test/e2e/puppeteer.js
'webgl_shaders_tonemapping',
'webgpu_nodes_playground',

// Unknown
// TODO: most of these can be fixed just by increasing idleTime and parseTime
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same with these.

@LeviPesin LeviPesin changed the title Puppeteer E2E test: Regenerate high-differenсe screenshots Puppeteer E2E test: Regenerate high-differenсe screenshots and decrease threshold Feb 2, 2023
@LeviPesin LeviPesin marked this pull request as ready for review February 2, 2023 09:15
@LeviPesin LeviPesin mentioned this pull request Feb 2, 2023
Merged
mrdoob
mrdoob reviewed Feb 3, 2023
View reviewed changes
test/e2e/puppeteer.js Outdated
@@ -59,7 +76,7 @@ const chromiumChannel = 'stable'; // stable -> beta -> dev -> canary (Mac and Wi

const port = 1234;
const pixelThreshold = 0.1; // threshold error in one pixel
const maxFailedPixels = 0.05; // at most 5% failed pixels
const maxFailedPixels = 0.3; // at most 0.3% failed pixels
Copy link
Owner

Choose a reason for hiding this comment

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

0.3% or 3%?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

0.3% -- more than 10x decrease.

@LeviPesin
Copy link
Contributor Author

@mrdoob Can you please merge this PR (and #25386)? Both these PRs make the test more precise -- and also after their merge I will be able to make and test a PR fixing most of the examples in the exception list.

@mrdoob mrdoob added this to the r150 milestone Feb 6, 2023
@mrdoob mrdoob merged commit 780feac into mrdoob:dev Feb 6, 2023
@LeviPesin LeviPesin deleted the puppeteer-regenerate-high-difference-screenshots branch February 6, 2023 09:51
This was referenced Feb 6, 2023
Merged
Draft
@LeviPesin LeviPesin restored the puppeteer-regenerate-high-difference-screenshots branch February 6, 2023 14:50
@LeviPesin LeviPesin deleted the puppeteer-regenerate-high-difference-screenshots branch February 6, 2023 15:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mrdoob mrdoob mrdoob left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
r150
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@LeviPesin @mrdoob