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

[angle] Update to chromium/4472 #18432

Merged
merged 2 commits into from
Jul 20, 2021
Merged

[angle] Update to chromium/4472 #18432

merged 2 commits into from
Jul 20, 2021

Conversation

past-due
Copy link
Contributor

@past-due past-due commented Jun 13, 2021 •
edited
Loading

Update the angle port (libANGLE) to chromium/4472 (corresponding to the current Chromium Stable release).

(This follows the recommendations in the libANGLE "Choosing an ANGLE branch" guide, matching the current Chromium Stable 91.0.4472.101.)

  • Which triplets are supported/not supported? Have you updated the CI baseline?

    Same triplets as existing

@past-due past-due force-pushed the angle_update_chromium_4472 branch 4 times, most recently from c795ed0 to 045bad3 Compare June 14, 2021 02:29
Neumann-A
Neumann-A reviewed Jun 14, 2021
View reviewed changes
ports/angle/CMakeLists.txt
Comment on lines +45 to +55
if(WIN32)
add_definitions(-DANGLE_IS_WIN)
endif()
if(LINUX)
add_definitions(-DANGLE_IS_LINUX)
endif()
if(ANGLE_IS_64_BIT_CPU)
add_definitions(-DANGLE_IS_64_BIT_CPU)
elseif(ANGLE_IS_32_BIT_CPU)
add_definitions(-DANGLE_IS_32_BIT_CPU)
endif()
Copy link
Contributor

Choose a reason for hiding this comment

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

Are those definitions only used when building or are they also needed by a consumer? (are the some header ifdefs pragmas)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

They do not appear to be exposed in any public headers. 👍

@past-due
Copy link
Contributor Author

past-due commented Jun 14, 2021 •
edited
Loading

The x64_osx/x64_linux CI runs are failing on the opencl port, which appears to be unrelated to this PR?

@past-due past-due marked this pull request as ready for review June 14, 2021 14:22
@JackBoosY JackBoosY added the category:port-update The issue is with a library, which is requesting update new revision label Jun 15, 2021
@JackBoosY
Copy link
Contributor

JackBoosY commented Jun 16, 2021 •
edited
Loading

@past-due That's related. Some files installed by angle are conflicts with opencl:

Starting package 60/106: opencl:x64-linux
Building package opencl[core]:x64-linux...
Building package opencl[core]:x64-linux... done
Installing package opencl[core]:x64-linux...
The following files are already installed in /mnt/vcpkg-ci/installed/x64-linux and are in conflict with opencl:x64-linux

Installed by angle:x64-linux
    include/CL/cl.h
    include/CL/cl_d3d10.h
    include/CL/cl_d3d11.h
    include/CL/cl_dx9_media_sharing.h
    include/CL/cl_dx9_media_sharing_intel.h
    include/CL/cl_egl.h
    include/CL/cl_ext.h
    include/CL/cl_ext_intel.h
    include/CL/cl_gl.h
    include/CL/cl_gl_ext.h
    include/CL/cl_platform.h
    include/CL/cl_va_api_media_sharing_intel.h
    include/CL/cl_version.h
    include/CL/opencl.h

Please remove them when installing angle.

Neumann-A
Neumann-A reviewed Jun 16, 2021
View reviewed changes
ports/angle/CMakeLists.txt
@@ -435,7 +683,7 @@ SET_TARGET_PROPERTIES(libANGLE PROPERTIES PREFIX "")
SET_TARGET_PROPERTIES(libGLESv2 PROPERTIES PREFIX "")
SET_TARGET_PROPERTIES(libEGL PROPERTIES PREFIX "")

install(TARGETS libEGL libGLESv2 libANGLE angle_common angle_image_util angle_gpu_info_util angle_translator angle_preprocessor GL_VISIBILITY EXPORT ANGLEExport
install(TARGETS libEGL libGLESv2 libANGLE angle_common angle_compression_utils angle_image_util angle_gpu_info_util angle_translator angle_preprocessor GL_VISIBILITY EXPORT ANGLEExport
Copy link
Contributor

Choose a reason for hiding this comment

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

I still wonder if this is correct or if the main target libEGL libGLESv2 libANGLE should use $<TARGET_OBJECTS:helper_targets> instead so that they don't need to get installed

Copy link
Contributor Author

@past-due past-due Jul 12, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

I'm afraid I have not had a chance to test this suggestion. As-is it is working fine for our purposes. @JackBoosY, your thoughts? (Otherwise, this is ready to go.)

@JackBoosY JackBoosY added requires:vcpkg-team-review This PR or issue requires someone on the vcpkg team to take a further look. and removed requires:author-response labels Jul 12, 2021
@past-due
Copy link
Contributor Author

Updated to pick up google/angle@d15be77 which was pushed to the chromium/4472 libANGLE branch.

@strega-nil-ms
Copy link
Contributor

Thanks @past-due!

@strega-nil-ms strega-nil-ms merged commit 1c20782 into microsoft:master Jul 20, 2021
@mattleibow mattleibow mentioned this pull request Sep 13, 2021
Merged
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Neumann-A Neumann-A Neumann-A left review comments

@strega-nil-ms strega-nil-ms strega-nil-ms approved these changes

Assignees

@JackBoosY JackBoosY

Labels
category:port-update The issue is with a library, which is requesting update new revision requires:vcpkg-team-review This PR or issue requires someone on the vcpkg team to take a further look.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@past-due @JackBoosY @strega-nil-ms @Neumann-A