From 8981ed434afddbb1a7f33cb35a769917b89095cf Mon Sep 17 00:00:00 2001 From: Matthias Koefferlein Date: Sat, 17 Aug 2019 23:55:49 +0200 Subject: [PATCH 1/2] First fix for issue-306: some polygons are not recognized as rounded, more robust radius extraction. --- src/db/db/dbPolygonTools.cc | 14 ++- ...PolygonTools.cc => dbPolygonToolsTests.cc} | 114 ++++++++++++++++++ src/db/unit_tests/unit_tests.pro | 4 +- 3 files changed, 126 insertions(+), 6 deletions(-) rename src/db/unit_tests/{dbPolygonTools.cc => dbPolygonToolsTests.cc} (95%) diff --git a/src/db/db/dbPolygonTools.cc b/src/db/db/dbPolygonTools.cc index 5873ed3c5..6148e5db7 100644 --- a/src/db/db/dbPolygonTools.cc +++ b/src/db/db/dbPolygonTools.cc @@ -858,6 +858,7 @@ do_extract_rad_from_contour (typename db::polygon::polygon_contour_iterator f typename db::polygon::polygon_contour_iterator p2 = p1; const double cos_thr = 0.8; + const double acute_cos_thr = -0.8; const double circle_segment_thr = 2.5; // search for the first circle segment (where cos(a) > cos_thr) @@ -1031,7 +1032,7 @@ do_extract_rad_from_contour (typename db::polygon::polygon_contour_iterator f if (in_corner) { std::pair > cp = elast.cut_point (e); - if (! cp.first) { + if (! cp.first || db::sprod (elast, e) < acute_cos_thr * elast.double_length () * e.double_length ()) { // We have a full 180 degree bend without a stop (actually two corners). // Use the segment in between that is perpendicular to the start and end segment as stop edge. @@ -1048,7 +1049,7 @@ do_extract_rad_from_contour (typename db::polygon::polygon_contour_iterator f } e = db::edge (*pp1, *pp2); - if (db::sprod_sign (elast, e) == 0) { + if (db::sprod_sign (elast, e) <= 0) { break; } @@ -1059,6 +1060,13 @@ do_extract_rad_from_contour (typename db::polygon::polygon_contour_iterator f } + ++nseg_part; + + if (nseg_part >= nseg) { + // not a valid rounded bend - skip this solution + return false; + } + cp = elast.cut_point (e); if (! cp.first) { return false; @@ -1068,8 +1076,6 @@ do_extract_rad_from_contour (typename db::polygon::polygon_contour_iterator f new_pts->push_back (cp.second); } - ++nseg_part; - asum -= asum_part; asum -= db::vprod (e.p1 () - db::point (), e.p2 () - db::point ()); nseg -= nseg_part; diff --git a/src/db/unit_tests/dbPolygonTools.cc b/src/db/unit_tests/dbPolygonToolsTests.cc similarity index 95% rename from src/db/unit_tests/dbPolygonTools.cc rename to src/db/unit_tests/dbPolygonToolsTests.cc index 5e6d4fb9b..9c9ebfd29 100644 --- a/src/db/unit_tests/dbPolygonTools.cc +++ b/src/db/unit_tests/dbPolygonToolsTests.cc @@ -1555,6 +1555,120 @@ TEST(204) EXPECT_EQ (pr.to_string (), "(0,0;0,40000;40000,40000;40000,0/10000,10000;30000,10000;30000,30000;10000,30000)"); } +// rounding +TEST(205_issue318) +{ + db::Point pattern [] = { + db::Point (0, 0), + db::Point (0, 420000), + db::Point (400000, 400000), + db::Point (400000, 0), + }; + + db::Polygon p; + p.assign_hull (&pattern[0], &pattern[0] + sizeof (pattern) / sizeof (pattern[0])); + + double rinner = 0.0, router = 0.0; + unsigned int n; + db::Polygon pr; + db::Polygon pp = compute_rounded (p, 100000, 200000, 64); + EXPECT_EQ (extract_rad (pp, rinner, router, n, &pr), true); + + EXPECT_EQ (tl::to_string (rinner), "0"); + EXPECT_EQ (tl::to_string (router), "200000"); + EXPECT_EQ (tl::to_string (n), "64"); + // slight rounding errors, but still a good approximation ... + EXPECT_EQ (pr.to_string (), "(0,0;0,419998;400000,400002;400000,0)"); + + pp = compute_rounded (p, 50000, 100000, 64); + EXPECT_EQ (extract_rad (pp, rinner, router, n, &pr), true); + + EXPECT_EQ (tl::to_string (rinner), "0"); + EXPECT_EQ (tl::to_string (router), "100000"); + EXPECT_EQ (tl::to_string (n), "64"); + // slight rounding issue due to ... + EXPECT_EQ (pr.to_string (), "(0,0;0,420001;400000,400000;400000,0)"); +} + +// rounding +TEST(206_issue318) +{ + db::Point pattern [] = { + db::Point (0, 0), + db::Point (0, 40000000), + db::Point (400000, 400000), + db::Point (400000, 0), + }; + + db::Polygon p; + p.assign_hull (&pattern[0], &pattern[0] + sizeof (pattern) / sizeof (pattern[0])); + + double rinner = 0.0, router = 0.0; + unsigned int n; + db::Polygon pr; + db::Polygon pp = compute_rounded (p, 100000, 200000, 64); + EXPECT_EQ (extract_rad (pp, rinner, router, n, &pr), true); + + EXPECT_EQ (tl::to_string (rinner), "0"); + EXPECT_EQ (tl::to_string (router), "199992"); + EXPECT_EQ (tl::to_string (n), "65"); + // good approximation of a top edge ... + EXPECT_EQ (pr.to_string (), "(0,0;0,618467;400000,581242;400000,0)"); + + pp = compute_rounded (p, 50000, 100000, 64); + EXPECT_EQ (extract_rad (pp, rinner, router, n, &pr), true); + + EXPECT_EQ (tl::to_string (rinner), "0"); + EXPECT_EQ (tl::to_string (router), "100000"); + EXPECT_EQ (tl::to_string (n), "64"); + // the acute corner is split into two parts + EXPECT_EQ (pr.to_string (), "(0,0;0,20309228;199083,20290710;400000,400000;400000,0)"); +} + +// rounding +TEST(207_issue318) +{ + db::Point pattern [] = { + db::Point(-2523825, -4693678), + db::Point(-2627783, -4676814), + db::Point(-2705532, -4629488), + db::Point(-2747861, -4559084), + db::Point(-2750596, -4499543), + db::Point(-2753284, -4335751), + db::Point(-2764621, -4271381), + db::Point(-2828260, -4154562), + db::Point(-2808940, -4144038), + db::Point(-2743579, -4264019), + db::Point(-2731316, -4333649), + db::Point(-2728604, -4498857), + db::Point(-2726139, -4552516), + db::Point(-2689468, -4613512), + db::Point(-2620017, -4655786), + db::Point(-2529175, -4670522), + db::Point(-2468652, -4627768), + db::Point(-2437469, -4536777), + db::Point(-2434902, -4384723), + db::Point(-2436252, -4320529), + db::Point(-2395450, -4234678), + db::Point(-2338494, -4144716), + db::Point(-2319906, -4156484), + db::Point(-2376150, -4245322), + db::Point(-2414148, -4325271), + db::Point(-2412898, -4384677), + db::Point(-2415531, -4540623), + db::Point(-2450148, -4641632) + }; + + db::Polygon p; + p.assign_hull (&pattern[0], &pattern[0] + sizeof (pattern) / sizeof (pattern[0])); + + double rinner = 0.0, router = 0.0; + unsigned int n; + db::Polygon pr; + // this polygon should not be recognized as rounded - it kind of looks like ... + EXPECT_EQ (extract_rad (p, rinner, router, n, &pr), false); +} + // is_convex TEST(300) { diff --git a/src/db/unit_tests/unit_tests.pro b/src/db/unit_tests/unit_tests.pro index f3e14367f..68b975953 100644 --- a/src/db/unit_tests/unit_tests.pro +++ b/src/db/unit_tests/unit_tests.pro @@ -37,7 +37,6 @@ SOURCES = \ dbPCells.cc \ dbPoint.cc \ dbPolygon.cc \ - dbPolygonTools.cc \ dbPropertiesRepository.cc \ dbRegion.cc \ dbShapeArray.cc \ @@ -72,7 +71,8 @@ SOURCES = \ dbDeepEdgePairsTests.cc \ dbNetlistCompareTests.cc \ dbNetlistReaderTests.cc \ - dbLayoutVsSchematicTests.cc + dbLayoutVsSchematicTests.cc \ + dbPolygonToolsTests.cc INCLUDEPATH += $$TL_INC $$DB_INC $$GSI_INC DEPENDPATH += $$TL_INC $$DB_INC $$GSI_INC From 2cc6909d2c45f9d4ae8fb8cb52df602e51713091 Mon Sep 17 00:00:00 2001 From: Matthias Koefferlein Date: Sun, 18 Aug 2019 17:25:28 +0200 Subject: [PATCH 2/2] Second fix for issue #306 (round function problem) This fix adds a "amend" option to the rounded corners dialog - disabling this option allows to skip the "undo rounding" step in case the algorithm does not determine the rounding properties of the input properly. Without "amend" enabled, the rounding will always be applied atop of any existing rounding. --- src/edt/edt/RoundCornerOptionsDialog.ui | 139 +++++++++++++----------- src/edt/edt/edtDialogs.cc | 46 ++++++-- src/edt/edt/edtDialogs.h | 8 +- src/edt/edt/edtMainService.cc | 98 +++++++++++++---- src/edt/edt/edtMainService.h | 18 +++ 5 files changed, 216 insertions(+), 93 deletions(-) diff --git a/src/edt/edt/RoundCornerOptionsDialog.ui b/src/edt/edt/RoundCornerOptionsDialog.ui index bd9144a68..1a43d2892 100644 --- a/src/edt/edt/RoundCornerOptionsDialog.ui +++ b/src/edt/edt/RoundCornerOptionsDialog.ui @@ -1,70 +1,78 @@ - + + RoundCornerOptionsDialog - - + + 0 0 469 - 241 + 271 - + Dialog - - - - - - - - Outer corner radius + + + + + Amend mode (undo existing rounding before applying new one) - - - + + + Number of points (for full circle) - - - + + + Inner corner radius - - + + - - + + - - - + + + + Outer corner radius + + + + + + + + + Qt::Horizontal - - - + + + Radius to apply on polygon corners (Radius for inner corners can be specified separately. Leave empty to get the same radius than for outer corners) - - - + + + Qt::Vertical - + 448 11 @@ -72,29 +80,29 @@ Leave empty to get the same radius than for outer corners) - - - + + + micron - - - + + + micron - - - + + + Qt::Vertical - + QSizePolicy::Fixed - + 20 10 @@ -102,28 +110,33 @@ Leave empty to get the same radius than for outer corners) - - - + + + Qt::Horizontal - + QDialogButtonBox::Cancel|QDialogButtonBox::Ok + + + + Qt::Vertical + + + QSizePolicy::Fixed + + + + 20 + 5 + + + + - router_le - label - label_2 - label_3 - rinner_le - points_le - line - label_4 - label_5 - label_6 - buttonBox @@ -133,11 +146,11 @@ Leave empty to get the same radius than for outer corners) RoundCornerOptionsDialog accept() - + 248 254 - + 157 274 @@ -149,11 +162,11 @@ Leave empty to get the same radius than for outer corners) RoundCornerOptionsDialog reject() - + 316 260 - + 286 274 diff --git a/src/edt/edt/edtDialogs.cc b/src/edt/edt/edtDialogs.cc index b1a932cba..ae20ad27e 100644 --- a/src/edt/edt/edtDialogs.cc +++ b/src/edt/edt/edtDialogs.cc @@ -414,11 +414,13 @@ MakeCellOptionsDialog::button_clicked () // RoundCornerOptionsDialog implementation RoundCornerOptionsDialog::RoundCornerOptionsDialog (QWidget *parent) - : QDialog (parent), mp_layout (0) + : QDialog (parent), mp_layout (0), m_router_extracted (0.0), m_rinner_extracted (0.0), m_npoints_extracted (64), m_has_extracted (false) { setObjectName (QString::fromUtf8 ("round_corners_options_dialog")); Ui::RoundCornerOptionsDialog::setupUi (this); + + connect (amend_cb, SIGNAL (stateChanged (int)), this, SLOT (amend_changed ())); } RoundCornerOptionsDialog::~RoundCornerOptionsDialog () @@ -426,21 +428,51 @@ RoundCornerOptionsDialog::~RoundCornerOptionsDialog () // .. nothing yet .. } -bool -RoundCornerOptionsDialog::exec_dialog (const db::Layout &layout, double &router, double &rinner, unsigned int &npoints) +void +RoundCornerOptionsDialog::amend_changed () { + if (amend_cb->isChecked () && m_has_extracted) { + router_le->setText (tl::to_qstring (tl::to_string (m_router_extracted))); + if (db::coord_traits::equal (m_router_extracted, m_rinner_extracted)) { + rinner_le->setText (QString ()); + } else { + rinner_le->setText (tl::to_qstring (tl::to_string (m_rinner_extracted))); + } + points_le->setText (tl::to_qstring (tl::to_string (m_npoints_extracted))); + } +} + +bool +RoundCornerOptionsDialog::exec_dialog (const db::Layout &layout, double &router, double &rinner, unsigned int &npoints, bool &undo_before_apply, double router_extracted, double rinner_extracted, unsigned int npoints_extracted, bool has_extracted) +{ + m_router_extracted = router_extracted; + m_rinner_extracted = rinner_extracted; + m_npoints_extracted = npoints_extracted; + m_has_extracted = has_extracted; + + amend_cb->blockSignals (true); + amend_cb->setEnabled (has_extracted); + amend_cb->setChecked (undo_before_apply && has_extracted); + amend_cb->blockSignals (false); + mp_layout = &layout; - router_le->setText (tl::to_qstring (tl::to_string (router))); - if (fabs (router - rinner) < 1e-6) { + double ro = undo_before_apply && has_extracted ? router_extracted : router; + double ri = undo_before_apply && has_extracted ? rinner_extracted : rinner; + unsigned int n = undo_before_apply && has_extracted ? npoints_extracted : npoints; + + router_le->setText (tl::to_qstring (tl::to_string (ro))); + if (db::coord_traits::equal (ro, ri)) { rinner_le->setText (QString ()); } else { - rinner_le->setText (tl::to_qstring (tl::to_string (rinner))); + rinner_le->setText (tl::to_qstring (tl::to_string (ri))); } - points_le->setText (tl::to_qstring (tl::to_string (npoints))); + points_le->setText (tl::to_qstring (tl::to_string (n))); if (QDialog::exec ()) { + undo_before_apply = m_has_extracted && amend_cb->isChecked (); + tl::from_string (tl::to_string (router_le->text ()), router); if (rinner_le->text ().isEmpty ()) { rinner = router; diff --git a/src/edt/edt/edtDialogs.h b/src/edt/edt/edtDialogs.h index 165cc650a..e7d8d6258 100644 --- a/src/edt/edt/edtDialogs.h +++ b/src/edt/edt/edtDialogs.h @@ -173,12 +173,18 @@ public: RoundCornerOptionsDialog (QWidget *parent); ~RoundCornerOptionsDialog (); - bool exec_dialog (const db::Layout &layout, double &rhull, double &rholes, unsigned int &npoints); + bool exec_dialog (const db::Layout &layout, double &router, double &rinner, unsigned int &npoints, bool &undo_before_apply, double router_extracted, double rinner_extracted, unsigned int npoints_extracted, bool has_extracted); virtual void accept (); +private slots: + void amend_changed (); + private: const db::Layout *mp_layout; + double m_router_extracted, m_rinner_extracted; + unsigned int m_npoints_extracted; + bool m_has_extracted; }; } // namespace edt diff --git a/src/edt/edt/edtMainService.cc b/src/edt/edt/edtMainService.cc index 5e572fc5b..67d3fd24d 100644 --- a/src/edt/edt/edtMainService.cc +++ b/src/edt/edt/edtMainService.cc @@ -60,7 +60,13 @@ MainService::MainService (db::Manager *manager, lay::LayoutView *view, lay::Plug m_align_hmode (0), m_align_vmode (0), m_align_visible_layers (false), m_origin_mode_x (-1), m_origin_mode_y (-1), m_origin_visible_layers_for_bbox (false), m_array_a (0.0, 1.0), m_array_b (1.0, 0.0), - m_array_na (1), m_array_nb (1) + m_array_na (1), m_array_nb (1), + m_router (0.0), m_rinner (0.0), m_npoints (64), m_undo_before_apply (true), + mp_round_corners_dialog (0), + mp_align_options_dialog (0), + mp_flatten_inst_options_dialog (0), + mp_make_cell_options_dialog (0), + mp_make_array_options_dialog (0) { // .. nothing yet .. } @@ -70,7 +76,52 @@ MainService::~MainService () // .. nothing yet .. } -void +edt::RoundCornerOptionsDialog * +MainService::round_corners_dialog () +{ + if (! mp_round_corners_dialog) { + mp_round_corners_dialog = new edt::RoundCornerOptionsDialog (view ()); + } + return mp_round_corners_dialog; +} + +edt::AlignOptionsDialog * +MainService::align_options_dialog () +{ + if (! mp_align_options_dialog) { + mp_align_options_dialog = new edt::AlignOptionsDialog (view ()); + } + return mp_align_options_dialog; +} + +lay::FlattenInstOptionsDialog * +MainService::flatten_inst_options_dialog () +{ + if (! mp_flatten_inst_options_dialog) { + mp_flatten_inst_options_dialog = new lay::FlattenInstOptionsDialog (view (), false /*don't allow prunining*/); + } + return mp_flatten_inst_options_dialog; +} + +edt::MakeCellOptionsDialog * +MainService::make_cell_options_dialog () +{ + if (! mp_make_cell_options_dialog) { + mp_make_cell_options_dialog = new edt::MakeCellOptionsDialog (view ()); + } + return mp_make_cell_options_dialog; +} + +edt::MakeArrayOptionsDialog * +MainService::make_array_options_dialog () +{ + if (! mp_make_array_options_dialog) { + mp_make_array_options_dialog = new edt::MakeArrayOptionsDialog (view ()); + } + return mp_make_array_options_dialog; +} + +void MainService::menu_activated (const std::string &symbol) { if (symbol == "edt::descend") { @@ -318,9 +369,7 @@ MainService::cm_flatten_insts () tl_assert (view ()->is_editable ()); check_no_guiding_shapes (); - lay::FlattenInstOptionsDialog options_dialog (view (), false /*don't allow prunining*/); - - if (options_dialog.exec_dialog (m_flatten_insts_levels, m_flatten_prune) && m_flatten_insts_levels != 0) { + if (flatten_inst_options_dialog ()->exec_dialog (m_flatten_insts_levels, m_flatten_prune) && m_flatten_insts_levels != 0) { view ()->cancel_edits (); @@ -908,11 +957,9 @@ MainService::cm_make_cell () if (cv_index >= 0) { - MakeCellOptionsDialog dialog (view ()); - const lay::CellView &cv = view ()->cellview (cv_index); - if (dialog.exec_dialog (cv->layout (), m_make_cell_name, m_origin_mode_x, m_origin_mode_y)) { + if (make_cell_options_dialog ()->exec_dialog (cv->layout (), m_make_cell_name, m_origin_mode_x, m_origin_mode_y)) { // Compute the selection's bbox to establish a good origin for the new cell db::Box selection_bbox; @@ -1231,9 +1278,10 @@ MainService::cm_convert_to_pcell () } } -static void extract_rad (std::vector &poly, double &rinner, double &router, unsigned int &n) +static bool extract_rad (std::vector &poly, double &rinner, double &router, unsigned int &n) { std::vector new_pts; + bool any_extracted = false; for (std::vector::iterator p = poly.begin (); p != poly.end (); ++p) { @@ -1246,6 +1294,7 @@ static void extract_rad (std::vector &poly, double &rinner, double new_poly.assign_hull (p->begin_hull (), p->end_hull (), false /*don't compress*/); } else { new_poly.assign_hull (new_pts.begin (), new_pts.end (), true /*compress*/); + any_extracted = true; } for (unsigned int h = 0; h < p->holes (); ++h) { @@ -1257,13 +1306,16 @@ static void extract_rad (std::vector &poly, double &rinner, double new_poly.insert_hole (p->begin_hole (h), p->end_hole (h), false /*don't compress*/); } else { new_poly.insert_hole (new_pts.begin (), new_pts.end (), true /*compress*/); + any_extracted = true; } } p->swap (new_poly); - } + } + + return any_extracted; } void @@ -1310,15 +1362,17 @@ MainService::cm_round_corners () // prepare: merge to remove cutlines and smooth to remove effects of cutlines db::EdgeProcessor ep; - std::vector out; - ep.merge (primary, out, 0 /*min_wc*/, false /*resolve holes*/, true /*min coherence*/); - for (std::vector ::iterator p = out.begin (); p != out.end (); ++p) { + std::vector in; + ep.merge (primary, in, 0 /*min_wc*/, false /*resolve holes*/, true /*min coherence*/); + for (std::vector ::iterator p = in.begin (); p != in.end (); ++p) { *p = smooth (*p, 1); } + std::vector out = in; + unsigned int n = 100; double rinner = 0.0, router = 0.0; - extract_rad (out, rinner, router, n); + bool has_extracted = extract_rad (out, rinner, router, n); const lay::CellView &cv = view ()->cellview (cv_index); double dbu = cv->layout ().dbu (); @@ -1326,13 +1380,16 @@ MainService::cm_round_corners () rinner *= dbu; router *= dbu; - RoundCornerOptionsDialog dialog (view ()); - if (! dialog.exec_dialog (cv->layout (), router, rinner, n)) { + if (! round_corners_dialog ()->exec_dialog (cv->layout (), m_router, m_rinner, m_npoints, m_undo_before_apply, router, rinner, n, has_extracted)) { return; } + if (! m_undo_before_apply || ! has_extracted) { + out.swap (in); + } + for (std::vector ::iterator p = out.begin (); p != out.end (); ++p) { - *p = compute_rounded (*p, rinner / dbu, router / dbu, n); + *p = compute_rounded (*p, m_rinner / dbu, m_router / dbu, m_npoints); } // remove holes (result in primary) @@ -1700,8 +1757,7 @@ MainService::cm_align () std::vector edt_services = view ()->get_plugins (); - AlignOptionsDialog dialog (view ()); - if (! dialog.exec_dialog (view (), m_align_hmode, m_align_vmode, m_align_visible_layers)) { + if (! align_options_dialog ()->exec_dialog (view (), m_align_hmode, m_align_vmode, m_align_visible_layers)) { return; } @@ -1797,9 +1853,7 @@ MainService::cm_make_array () throw tl::Exception (tl::to_string (QObject::tr ("Nothing selected to make arrays of"))); } - MakeArrayOptionsDialog dialog (view ()); - - if (dialog.exec_dialog (m_array_a, m_array_na, m_array_b, m_array_nb)) { + if (make_array_options_dialog ()->exec_dialog (m_array_a, m_array_na, m_array_b, m_array_nb)) { view ()->cancel_edits (); diff --git a/src/edt/edt/edtMainService.h b/src/edt/edt/edtMainService.h index 67aa8755d..3c6331c08 100644 --- a/src/edt/edt/edtMainService.h +++ b/src/edt/edt/edtMainService.h @@ -39,6 +39,7 @@ namespace lay { class PluginRoot; + class FlattenInstOptionsDialog; } namespace edt { @@ -46,6 +47,10 @@ namespace edt { class Service; class EditorOptionsPages; class EditorOptionsPage; +class RoundCornerOptionsDialog; +class MakeCellOptionsDialog; +class MakeArrayOptionsDialog; +class AlignOptionsDialog; // ------------------------------------------------------------- @@ -205,9 +210,22 @@ private: bool m_origin_visible_layers_for_bbox; db::DVector m_array_a, m_array_b; unsigned int m_array_na, m_array_nb; + double m_router, m_rinner; + unsigned int m_npoints; + bool m_undo_before_apply; + edt::RoundCornerOptionsDialog *mp_round_corners_dialog; + edt::AlignOptionsDialog *mp_align_options_dialog; + lay::FlattenInstOptionsDialog *mp_flatten_inst_options_dialog; + edt::MakeCellOptionsDialog *mp_make_cell_options_dialog; + edt::MakeArrayOptionsDialog *mp_make_array_options_dialog; void boolean_op (int mode); void check_no_guiding_shapes (); + edt::RoundCornerOptionsDialog *round_corners_dialog (); + edt::AlignOptionsDialog *align_options_dialog (); + lay::FlattenInstOptionsDialog *flatten_inst_options_dialog (); + edt::MakeCellOptionsDialog *make_cell_options_dialog (); + edt::MakeArrayOptionsDialog *make_array_options_dialog (); }; }