-
Notifications
You must be signed in to change notification settings - Fork 415
Search Update + NoC GUI Update #2102
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.
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
Conversation
|
Needs a make format too. |
Read the docs error was an infrastructure issue (now fixed). Can ignore. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some commenting changes suggested and a few minor restructurings. Looks very good and clean overall!
vpr/src/draw/search_bar.cpp
Outdated
if (type && type[0] == '\0') { | ||
warning_dialog_box("Please select a search type"); | ||
app->refresh_drawing(); | ||
return "Failed lol"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure this is the best failure code ... empty string instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh oops forgot to get rid of this. Set it to empty string and added checks where it is called
One warning in vpr that should be fixed. |
@jmah76 @SebastianLievano : this looks good to merge except the "error on warning" compiler check is failing for vpr. |
I think the pin_criticality + 1 warnings were there before I started (not 100% sure, but I haven't touched route_timing.cpp, the file where the warnings are popping up). Those "errors" are suppressed, but I can try to look into this on a different branch (branch route_timing_offset) |
OK, if those errors are suppressed and the test passes once the tptr error is gone, I'm OK with it. Getting rid of them would be even cleaner if you can though! If you expand the array/vector to have a 0'th entry, please set that 0'th entry to invalid values (NULL for a pointer, -1 for an integer, etc.) so we are likely to crash if we accidentally use it. |
I made the changes here. I was able to get rid of the warnings by adding an entry in the arrays that were throwing the warnings. Their "zeroth" elements are either -1 (for the float and int arrays) or nullptr (for the ptr arrays). Once the CI tests pass, I'll make a PR. |
Great, thanks Jennifer. |
Added final modifications to toggle completion off after first showing. However, some limitation either in EZGL or GTK (probably some bug with focuses) means some key presses do not register in the function, making the toggling a bit inconsistent. Not worth pursuing imho. Will merge once checks are done @vaughnbetz |
One failure, but looks like an unrelated infrastructure issue. Can you rerun that test and see if it passes now (instructions are in the developer guide in the web documentation)? |
**Search **
Updated search functionality to include Block and Net Name suggestions/autocomplete, toggleable by hitting the "Enter" or "Return" key.
Additionally, switched search to include Cluster and Atom netlist for Blocks, and the Atom netlist for nets (Cluster netlist is a subset of atom). The autosuggest algorithm/set up is meant to only run once when you hit enter, so you may need to rehit the key to reactivate it. Upon hitting enter, the program will search through all of the names and display all of the names that contain the given substring. This is obviously a large operation, and will take a bit of time.
Furthermore, searching a subblock now highlights the block if it is visible at the current zoom lvl, and if not, highlights the containing cluster block. Additionally, a new subroutine which highlights an Atom Block from its id has been added, could be useful.
NoC Graphics Additions
Added an NoC graphics combo box in the Block Settings dropdown. This will only display if the user uses the "--noc on" option on startup.
Motivation and Context
The original search function had no form of automatic completion, required users to enter the full names of the block or net they were searching for, with no room for error, and was also fully based on the Clustered netlist, and thus could not search or highlight internals. It was pretty difficult to use as a result, as you'd need to know the full name of the thing you were searching for, and the names are often synthesized by VTR so you probably would not know that.
The new functionality will automatically parse through all names to find those that contain the given string. This allows users to search for a specific string even if they don't know its full name, and they can enable auto-suggestion only when they need it by pressing enter.
How Has This Been Tested?
All new GUI features have been tested and ensured to perform as expected. Search functionality has also been tested a lot on different architectures of varying sizes.
Types of changes