Fix headless gui::dump_heatmap#10366
Conversation
Signed-off-by: naveenvenk17 <naveenvenkat1711@gmail.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a syncHeatMapChips method to ensure heatmap data sources are synchronized with the current database chip during headless or console sessions. This method is called within getHeatMap to maintain data integrity when the UI is not present. Additionally, a new integration test, dump_heatmap_headless, has been added to verify heatmap generation in these environments. Feedback suggests extending this synchronization logic to the getHeatMaps() method to ensure consistent behavior across the API.
| if (hasUI() || db_ == nullptr) { | ||
| return; | ||
| } | ||
|
|
||
| // Console and headless sessions do not receive MainWindow::setBlock(). | ||
| auto* chip = db_->getChip(); | ||
| for (auto* heat_map : heat_maps_) { | ||
| if (heat_map->getChip() != chip) { | ||
| heat_map->setChip(chip); | ||
| heat_map->destroyMap(); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
The syncHeatMapChips method correctly handles the synchronization of heatmap data sources with the current database chip in headless mode. However, for better API consistency and to ensure that any code iterating over all heatmaps (via getHeatMaps()) also sees the updated state, consider moving the implementation of getHeatMaps() from the header to the .cpp file and calling syncHeatMapChips() there as well.
Signed-off-by: naveenvenk17 <naveenvenkat1711@gmail.com>
|
Addressed Gemini's heatmap enumeration feedback in follow-up commit 66f1357 by moving getHeatMaps() out of the header and syncing chips there as well. |
Signed-off-by: naveenvenk17 <naveenvenkat1711@gmail.com>
Signed-off-by: naveenvenk17 <naveenvenkat1711@gmail.com>
|
|
|
clang-tidy review says "All clean, LGTM! 👍" |
Signed-off-by: naveenvenk17 <naveenvenkat1711@gmail.com>
|
clang-tidy review says "All clean, LGTM! 👍" |
|
Agreed. I pushed 56720db to make the regression exercise the real fix: it adds a C++ test against //src/gui:gui_qt_headless that initializes heatmaps before a chip exists, creates the chip, and verifies headless lookup syncs every source to the current chip. I also removed the misleading Bazel Tcl coverage that only used the stub GUI, while keeping the Tcl regression for CMake GUI builds. |
Signed-off-by: naveenvenk17 <naveenvenkat1711@gmail.com>
|
Follow-up: Clang-Format caught the include ordering in the new C++ regression. Fixed in ba6d2b9. |
|
clang-tidy review says "All clean, LGTM! 👍" |
|
Across the board build failures |
|
Orphaned |
2 participants
Summary
gui::dump_heatmapin console/headless sessions by syncing built-in heatmap sources with the current database chip when no QtMainWindowis active.gcd_nangate45.defwithout-guiand dumps thePlacementheatmap CSV.Type of Change
Impact
Headless GUI Tcl heatmap commands can populate maps after
read_def/read_db; Qt GUI behavior is unchanged because the main window still owns block/chip updates while active.Verification
./etc/Build.sh). Not run: this Windows/WSL worker does not have the OpenROAD build toolchain (cmake/bazel/compiler) available.git diff --cached --checkbefore commit; no whitespace errors)Additional local checks:
GUI-0072failure on installed WSL OpenROADv2.0-17598-ga008522d8with the same headlessgui::dump_heatmap Placementflow.tclshwith stubbed OpenROAD commands.Related Issues
Fixes #5897