Refactored vpr/src/draw #2049

jmah76 · 2022-06-02T17:48:44Z

Refactored the vpr/src/draw folder by splitting draw.cpp into smaller files to reduce file length.

Motivation and Context

Draw.cpp was too long (~3000-4000 lines), now much shorter (~1500 lines).

How It Was Tested

VPR Graphics was run once with the changes, then without. The visualizations of both were compared, and were checked if they were relatively the same.
Command line scripts in the documentation were tested for expected behaviour.
- Note: As in Issue --graphics command doesn't seem to work #2001, graphics_commands don't work (throws Unexpected command-line argument, could be that the commands aren't being recognized as graphics commands but normal command line options-- the error message matches that of an options error and not a graphics command error. I think it's because the OP of the options reading code forgot to add subcommands to the --graphics_command argument, which would be needed to collect whatever comes after --graphics_command. This is a bigger issue than I thought, so I'll continue this convo in Issue --graphics command doesn't seem to work #2001 ). This can be fixed in a seperate PR after this PR lands.
Buttons on the GUI were pressed and tested for expected behaviour.

This reverts commit e6f8641.

vaughnbetz

Nice refactoring; I have some further suggestions though.

vaughnbetz · 2022-06-03T21:22:55Z

packing_pin_util.rpt

@@ -0,0 +1,65 @@
+#Packing pin usage report


Is this file accidentally added? It looks like an output file that shouldn't go in the repo.

vaughnbetz · 2022-06-03T21:24:28Z

vpr/src/draw/draw.cpp

@@ -34,6 +34,13 @@
 #include "globals.h"
 #include "draw_color.h"
 #include "draw.h"
+#include "draw_basic.h"


Would be good to add to the top-level comment at the top of this file: say what kinds of functions should go in this file (or say put new functions in helper files if at all possible).

vaughnbetz · 2022-06-03T21:26:16Z

vpr/src/draw/draw.cpp

+ * could be caused by the user clicking on a routing resource, toggled, or
+ * fan-in/fan-out of a highlighted node.
+ */
+bool draw_if_net_highlighted(ClusterNetId inet) {


If there is no corresponding comment for this function in the .h file, duplicate or summarize this comment there (we're moving toward commenting what a function does in the .h file, and how it works in the .cpp file if necessary).

vaughnbetz · 2022-06-03T21:28:45Z

vpr/src/draw/draw.cpp

 //Returns the set of rr nodes which connect driver to sink
-static std::vector<int> trace_routed_connection_rr_nodes(
+std::vector<int> trace_routed_connection_rr_nodes(


Duplicate this comment (or move it) in the .h file.

vaughnbetz · 2022-06-03T21:29:06Z

vpr/src/draw/draw.cpp

@@ -3787,7 +944,7 @@ bool trace_routed_connection_rr_nodes_recurr(const t_rt_node* rt_node,
 }

 //Find the edge between two rr nodes
-static t_edge_size find_edge(int prev_inode, int inode) {
+t_edge_size find_edge(int prev_inode, int inode) {


Duplicate this comment in the .h file

vaughnbetz · 2022-06-03T21:39:06Z

vpr/src/draw/draw_toggle_functions.h

+#    include "place_macro.h"
+#    include "buttons.h"
+
+void toggle_nets(GtkWidget* /*widget*/, gint /*response_id*/, gpointer /*data*/);


Copy or move "what function does" comments from .cpp to the .h file where you can.

vaughnbetz · 2022-06-03T21:39:26Z

vpr/src/draw/draw_triangle.cpp

@@ -0,0 +1,122 @@
+/* draw_triangle.cpp contains functions that draw triangles.*/


Can you say what they are used for?

(They are generally used for showing switching in the routing, the direction of signals, and such.)

vaughnbetz · 2022-06-03T21:40:40Z

vpr/src/draw/draw_triangle.h

+#    include "place_macro.h"
+#    include "buttons.h"
+
+void draw_triangle_along_line(ezgl::renderer* g, ezgl::point2d start, ezgl::point2d end, float relative_position = 1., float arrow_size = DEFAULT_ARROW_SIZE);


Move function-level comments from .cpp file to here (the how to call and what do I do part).

vaughnbetz · 2022-06-03T21:41:50Z

vpr/src/draw/draw_types.h

@@ -163,6 +163,9 @@ typedef struct {
 * save_graphics_file_base: Base of save graphis file name (i.e. before extension)
 * pres_fac: present congestion cost factor
 */
+/* Note: t_draw_struct is used in the same way as a Context, but cannot be a Context because Contexts are
+ * not copyable, while t_draw_struct must be.


Say why it must be copied; graphics commands feature uses that to do save an restore of the drawing state.

vaughnbetz · 2022-06-03T21:44:44Z

vpr/src/draw/draw_xtoy.cpp

@@ -0,0 +1,571 @@
+/*draw_xtoy.cpp contains all functions that draw lines from one point to another.*/


I'd rename this file. I initially read it as being a "toy". It is really drawing the edges between routing resource (rr) nodes, so a better name would be
draw_rr_edges.cpp

vaughnbetz

Changes look good so merging. One nit on adding a space after /* and before */ in one spot at least.

vaughnbetz · 2022-06-09T16:23:39Z

vpr/src/draw/draw_mux.cpp

@@ -0,0 +1,133 @@
+/*draw_mux.cpp contains all functions that draw muxes.*/


Small nit, but good to add these spaces ...

jmah76 and others added 12 commits June 1, 2022 09:44

Initial commit

e6f8641

Revert "Initial commit"

83e71c5

This reverts commit e6f8641.

Initital commit

b2006c0

Moved one function to new file to test

2650d0d

Seperated draw rr functions into file

3c4140d

Moved functions that draw lines from point a to b into seperate file

a28e193

toggle functions moved to seperate file

2e53304

triangle drawing functions moved to seperate file

706381f

moved mux drawing functions to seperate file

4b08e11

moved drawing basic element functions to seperate file

ff50aac

Fixing formatting & removing unused headers included

d9f181b

Merge branch 'master' into draw_refactor

8c8099c

github-actions bot added the VPR VPR FPGA Placement & Routing Tool label Jun 2, 2022

jmah76 added 4 commits June 3, 2022 12:40

Added missed include

9078b96

.gitignore file changed

f4a4720

fixed .gitignore merge conflict

1c6c495

added comments to new files

881ca62

vaughnbetz requested changes Jun 3, 2022

View reviewed changes

jmah76 and others added 3 commits June 4, 2022 11:11

Delete packing_pin_util.rpt

419b5fa

Added comments

561d03d

Fixed alpha widget not changing transparency bug

edd4f69

vaughnbetz approved these changes Jun 9, 2022

View reviewed changes

vaughnbetz merged commit ba52c3a into master Jun 9, 2022

vaughnbetz deleted the draw_refactor branch June 9, 2022 16:24

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Refactored vpr/src/draw #2049

Refactored vpr/src/draw #2049

Uh oh!

jmah76 commented Jun 2, 2022 •

edited

Loading

Uh oh!

vaughnbetz left a comment

Uh oh!

vaughnbetz Jun 3, 2022

Uh oh!

vaughnbetz Jun 3, 2022

Uh oh!

vaughnbetz Jun 3, 2022

Uh oh!

vaughnbetz Jun 3, 2022

Uh oh!

vaughnbetz Jun 3, 2022

Uh oh!

vaughnbetz Jun 3, 2022

Uh oh!

vaughnbetz Jun 3, 2022

Uh oh!

vaughnbetz Jun 3, 2022

Uh oh!

vaughnbetz Jun 3, 2022

Uh oh!

vaughnbetz Jun 3, 2022

Uh oh!

vaughnbetz Jun 3, 2022

Uh oh!

vaughnbetz left a comment

Uh oh!

vaughnbetz Jun 9, 2022

Uh oh!

Uh oh!

		@@ -0,0 +1,122 @@
		/* draw_triangle.cpp contains functions that draw triangles.*/

		@@ -0,0 +1,571 @@
		/draw_xtoy.cpp contains all functions that draw lines from one point to another./

		@@ -0,0 +1,133 @@
		/draw_mux.cpp contains all functions that draw muxes./

Refactored vpr/src/draw #2049

Refactored vpr/src/draw #2049

Uh oh!

Conversation

jmah76 commented Jun 2, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation and Context

How It Was Tested

Uh oh!

vaughnbetz left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vaughnbetz left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jmah76 commented Jun 2, 2022 •

edited

Loading