-
Notifications
You must be signed in to change notification settings - Fork 428
Feature ap draw #3285
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
base: master
Are you sure you want to change the base?
Feature ap draw #3285
Conversation
e026e80
to
e53ab81
Compare
This will not break place and route drawing and it creates a initial world of different dimension than place and route drawing
No more segfault when clicking. Added after solve and after legalization draw.
skip set_initial_world when no display
a10f497
to
7eddb5c
Compare
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.
I am excited to get this in! I have added some comments regarding organization. I want to keep AP as clean as possible, without having confusing drawing code in the loop if we can.
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.
I would like to keep the drawing code pretty isolated from the AP code as much as possible. In the future, we may decide to make another global placer as well, and I would like to avoid duplicate code.
I suggest that you create a new class in the analytical_place
directory for drawing. Something like APDrawManager or something. This class would then have methods for drawing the pre and post legalized placements. This would remove the global accesses from this method. The global placer can then take this as an argument, which it can use to draw whenever it needs.
|
||
if (hpwl_relative_gap < target_hpwl_relative_gap_) | ||
break; | ||
|
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.
Why add these extra lines?
PartialPlacement best_p_placement(ap_netlist_); | ||
double best_ub_hpwl = std::numeric_limits<double>::max(); | ||
|
||
#ifndef NO_GRAPHICS |
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.
See my comments above. I would like to keep these ifdefs and the draw state out of this class.
draw_state->pic_on_screen = pic_on_screen_val; | ||
} | ||
|
||
// What is this? Always true! |
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.
I would mark this as a FIXME, or raise an issue so we can resolve. A comment like this will get missed.
#endif /* NO_GRAPHICS */ | ||
} | ||
|
||
#ifndef NO_GRAPHICS |
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.
I would be careful with this ifdef. You are not defining these functions when graphics is disabled. However, they are still available in the header file...
I would put the ifdef inside of the method and have the method do nothing when graphics is not enabled.
*/ | ||
void init_draw_coords(float clb_width, const BlkLocRegistry& blk_loc_registry); | ||
|
||
void set_initial_world_ap(); |
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.
These new methods need comments. Also see my prior comments on their definitions.
* Blocks are drawn in layer order (so that semi-transparent blocks/grids render well)*/ | ||
void drawplace(ezgl::renderer* g); | ||
|
||
void draw_analytical_place(ezgl::renderer* g); |
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.
Needs comment.
Now able to draw analytical placement
Description
Motivation and Context
To give more insight to AP flow
How Has This Been Tested?
By hand
Types of changes
Checklist: