gui: Enable gui with bazel#7725
Merged
maliberty merged 13 commits intoJul 15, 2025
Merged
Conversation
It is current off by default, and I would like to see others test it before we turn it on by default. In order to build with the gui you need to pass the platform flag to bazel build. `bazelisk build --//:platform=gui //:openroad` Signed-off-by: Ethan Mahintorabi <ethanmoon@google.com>
Collaborator
Author
|
@hzeller FYI |
Contributor
|
clang-tidy review says "All clean, LGTM! 👍" |
gadfort
requested changes
Jul 6, 2025
Contributor
gadfort
left a comment
gadfort
left a comment
There was a problem hiding this comment.
@QuantamHD I was unable to build this on WSL. Here are some of the warnings:
INFO: From Executing genrule @@qt-bazel+//qt_source:UIC__qtbase-6.9.1_src_widgets_dialogs_qfiledialog.ui:
Detected locale "C" with character encoding "ANSI_X3.4-1968", which is not UTF-8.
Qt depends on a UTF-8 locale, and has switched to "C.UTF-8" instead.
If this causes problems, reconfigure your locale. See the locale(1) manual
for more information.
INFO: From Generating MOC code from external/qt-bazel+/qt_source/qtcharts-6.9.1/src/charts/glwidget_p.h:
external/qt-bazel+/qt_source/qtcharts-6.9.1/src/charts/glwidget_p.h: note: No relevant classes found. No output generated.
Errors from linking:
Use --sandbox_debug to see verbose messages from the sandbox and retain the sandbox build root for debugging
ld.lld: error: undefined reference: __isoc23_strtol
>>> referenced by bazel-out/k8-fastbuild/bin/_solib_k8/_U_A_Aqt-bazel+_S_Sinterface_Ulibs_Sxkbcommon_Cxkbcommon_Uifso___Uinterface_Ulibs_Sxkbcommon/libxkbcommon.ifso (disallowed by --no-allow-shlib-undefined)
ld.lld: error: undefined reference: __isoc23_strtoul
>>> referenced by bazel-out/k8-fastbuild/bin/_solib_k8/_U_A_Aqt-bazel+_S_Sinterface_Ulibs_Sxkbcommon_Cxkbcommon_Uifso___Uinterface_Ulibs_Sxkbcommon/libxkbcommon.ifso (disallowed by --no-allow-shlib-undefined)
ld.lld: error: undefined reference: __isoc23_strtol
>>> referenced by bazel-out/k8-fastbuild/bin/_solib_k8/_U_A_Aqt-bazel+_S_Sinterface_Ulibs_Sxcb_Cxcb_Ucursor_Uifso___Uinterface_Ulibs_Sxcb/libxcb-cursor.ifso (disallowed by --no-allow-shlib-undefined)
ld.lld: error: undefined reference: __isoc23_sscanf
>>> referenced by bazel-out/k8-fastbuild/bin/_solib_k8/_U_A_Aqt-bazel+_S_Sinterface_Ulibs_SX11_Cx11_Uifso___Uinterface_Ulibs_SX11/libX11.ifso (disallowed by --no-allow-shlib-undefined)
ld.lld: error: undefined reference: __isoc23_strtol
>>> referenced by bazel-out/k8-fastbuild/bin/_solib_k8/_U_A_Aqt-bazel+_S_Sinterface_Ulibs_SX11_Cx11_Uifso___Uinterface_Ulibs_SX11/libX11.ifso (disallowed by --no-allow-shlib-undefined)
ld.lld: error: undefined reference: __isoc23_strtoul
>>> referenced by bazel-out/k8-fastbuild/bin/_solib_k8/_U_A_Aqt-bazel+_S_Sinterface_Ulibs_SX11_Cx11_Uifso___Uinterface_Ulibs_SX11/libX11.ifso (disallowed by --no-allow-shlib-undefined)
ld.lld: error: undefined reference: __isoc23_strtol
>>> referenced by bazel-out/k8-fastbuild/bin/_solib_k8/_U_A_Aqt-bazel+_S_Sinterface_Ulibs_SX11_Cx11_Uice_Uifso___Uinterface_Ulibs_SX11/libICE.ifso (disallowed by --no-allow-shlib-undefined)
ld.lld: error: undefined reference: arc4random_buf
>>> referenced by bazel-out/k8-fastbuild/bin/_solib_k8/_U_A_Aqt-bazel+_S_Sinterface_Ulibs_SX11_Cx11_Uice_Uifso___Uinterface_Ulibs_SX11/libICE.ifso (disallowed by --no-allow-shlib-undefined)
maliberty
reviewed
Jul 6, 2025
maliberty
reviewed
Jul 6, 2025
Member
|
Fails on qt5: |
Member
|
It built for me but how do you actually run it? |
Member
|
No luck with |
Collaborator
|
On Ubuntu 25.04 I get the error below: |
Collaborator
|
On Ubuntu 25.04, I can get it to build if I use the trick from #7349: But it doesn't run: If I run |
Collaborator
|
Trying something more advanced in Ubuntu 25.04: Lets ram that square peg into the round hole and see if we get further... Now it crashes at: |
Collaborator
|
Can't compile: apparently there is a pcre2.h dependency inside Qt that is not captured in the corresponding build file. |
Contributor
|
@QuantamHD on my ubuntu24 machine, I got the gui open, but it segfaulted the moment I tried to open a file: |
Signed-off-by: Ethan Mahintorabi <ethanmoon@google.com>
Contributor
|
clang-tidy review says "All clean, LGTM! 👍" |
…screen size on QT5 Signed-off-by: Ethan Mahintorabi <ethanmoon@google.com>
Collaborator
Author
|
@maliberty @gadfort @oharboe Please give it another go. |
Signed-off-by: Ethan Mahintorabi <ethanmoon@google.com>
Signed-off-by: Ethan Mahintorabi <ethanmoon@google.com>
Collaborator
Author
@hzeller This should be fixed. |
Contributor
|
clang-tidy review says "All clean, LGTM! 👍" |
2 similar comments
Contributor
|
clang-tidy review says "All clean, LGTM! 👍" |
Contributor
|
clang-tidy review says "All clean, LGTM! 👍" |
Collaborator
Author
|
As an FYI to everyone. This QT build is dynamically linking in X11 libs, because it's generally not advisable to statically link them. As a result on Ubuntu distributions you may run into an error complaining about libxcb-cursor. This is a common problem across all QT6 apps, since this PR also moves us to QT6 from QT5. You will need to install the runtime shared object. |
Signed-off-by: Ethan Mahintorabi <ethanmoon@google.com>
Contributor
|
@QuantamHD still getting the segfault I mention previously on Ubuntu24. But it does link on WSL ubuntu22. |
Signed-off-by: Ethan Mahintorabi <ethanmoon@google.com>
Signed-off-by: Ethan Mahintorabi <ethanmoon@google.com>
Contributor
|
clang-tidy review says "All clean, LGTM! 👍" |
Signed-off-by: Ethan Mahintorabi <ethanmoon@google.com>
Collaborator
Author
|
Okay @hzeller try now. I tried to remove the GL deps. |
Contributor
|
clang-tidy review says "All clean, LGTM! 👍" |
Collaborator
|
Still runs into that issue: qplatformintegration.h which includes qopenglcontext.h which in turn includes qopengl.h The qopenglcontext.h does have an |
Collaborator
|
There also seems to be a reference to |
Collaborator
|
Still fails: |
Collaborator
|
Worked using workaround: 
|
Collaborator
|
Tried on a larger design, megaboom. Worked! Perhaps merge (soon) as a starting point for further refinements & testing. I don't think we'll see a whole lot more testing until it is in master and enabled by default. I prefer working with some snags in the GUI and using a bazel build over cmake / sudo ./setup.sh hell (I don't have any specific snags that are holding me up, but I'd need to switch to day-to-day use). |
gadfort
reviewed
Jul 11, 2025
Comment on lines
+1552
to
+1554
| std::cout << "Your system has set QT_QPA_PLATFORM='" << qpa_platform | ||
| << "', openroad only supports 'offscreen' and 'xcb', please " | ||
| "include one of these plugins in your platform env\n"; |
Contributor
There was a problem hiding this comment.
I'm not sure if it matters, but it would be nice to format this like a logger message (in the case that this is called via gui::show the logger is ready.
Contributor
|
@oharboe that is great to hear. I would like to run this through its paces to make sure any obvious issues have I would encounter would appear before merging. Is the linking issue you seeing related to this PR? it seems to stem from abc, but maybe open an issue once this merges so it can be tracked. |
Contributor
|
@QuantamHD I tried saving an image and got: It doesn't happen when running via a script, but if I right click and save image from there it generated that error. |
Collaborator
I think we should worry mostly about regressions to CMake built OpenROAD and bazel CI before merging, given that this is gated by a flag. We can continue to chip away at things one at the time after merging. I added a feature request to put the bazel built openroad alongside cmake built in the docker image to make it easier to widen testing in CI and with bazel-orfs: #7790 |
Member
|
I'm ok to merge and refine in follow on PRs if that is ok with @QuantamHD We don't use OpenGL today but I could imagine using it in the future fwiw. |
Collaborator
Author
|
I'm fine to merge as is. |
maliberty
reviewed
Jul 11, 2025
Collaborator
|
Looks like one more reason to always IWYU instead of forward declaring. |
Member
No these are abstract types and they are never defined. |
…STA types Signed-off-by: Ethan Mahintorabi <ethanmoon@google.com>
Collaborator
Author
|
@maliberty Added the additional documentation. @gadfort I fixed the screenshot saving. |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Signed-off-by: Ethan Mahintorabi <ethanmoon@google.com>
Contributor
|
clang-tidy review says "All clean, LGTM! 👍" |
maliberty
approved these changes
Jul 15, 2025
gadfort
approved these changes
Jul 15, 2025
Contributor
|
@maliberty might be nice to add a build in the CI for this configuration |
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
It is current off by default, and I would like to see others test it before we turn it on by default.
In order to build with the gui you need to pass the platform flag to bazel build. The other major restriction is that only linux is enabled, additional qt work will be required to enable it for macos.
bazelisk build --//:platform=gui //:openroad@gadfort @oharboe @maliberty Please give it a test on your linux machines/WSL.
I also added a few ifdefs around the regex usage so that we can maintain both the bazel build and the cmake build which is stuck on qt5.