electron-rare/kicad-source-mirror
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Don't clear references unconditionally

Browse Source 
If we are clearing all references in the schematic, then we can clear
our references, otherwise, just eliminate the ones for the proper
sheets.   We don't want to clear the references on Sheet A when we copy
and paste it to Sheet B.

Fixes https://gitlab.com/kicad/code/kicad/-/issues/20173

(cherry picked from commit 8d37b3787d)
This commit is contained in:
Seth Hillbrand
2026-01-02 14:11:18 -08:00
parent cd8ecbcc6d
commit d2cd5ecdd1
8 changed files with 6407 additions and 9 deletions
Download Patch File Download Diff File Expand all files Collapse all files
eeschema/sch_symbol.cpp
+12 -9
View File
@@ -1665,16 +1665,19 @@ void SCH_SYMBOL::ClearAnnotation( const SCH_SHEET_PATH* aSheetPath, bool aResetP
for( std::unique_ptr<SCH_PIN>& pin : m_pins )
pin->ClearDefaultNetName( aSheetPath );
// These 2 changes do not work in complex hierarchy.
// When a clear annotation is made, the calling function must call a
// UpdateAllScreenReferences for the active sheet.
// But this call cannot made here.
wxString currentReference = m_fields[REFERENCE_FIELD].GetText();
// Only modify the REFERENCE field text when clearing ALL annotations (aSheetPath is NULL).
// When clearing for a specific sheet path, we must preserve the field text because it serves
// as a fallback for GetRef() when instances for other sheet paths are looked up.
// See issue #20173: modifying field text here corrupts references in shared screens.
if( !aSheetPath )
{
wxString currentReference = GetField( REFERENCE_FIELD )->GetText();
if( currentReference.IsEmpty() || aResetPrefix )
m_fields[REFERENCE_FIELD].SetText( UTIL::GetRefDesUnannotated( m_prefix ) );
else
m_fields[REFERENCE_FIELD].SetText( UTIL::GetRefDesUnannotated( currentReference ) );
if( currentReference.IsEmpty() || aResetPrefix )
GetField( REFERENCE_FIELD )->SetText( UTIL::GetRefDesUnannotated( m_prefix ) );
else
GetField( REFERENCE_FIELD)->SetText( UTIL::GetRefDesUnannotated( currentReference ) );
}
}
qa/data/eeschema/issue20173/Kicad 9 - multi channel test.kicad_pcb
+1727
View File
File diff suppressed because it is too large Load Diff
qa/data/eeschema/issue20173/Kicad 9 - multi channel test.kicad_sch
+1717
View File
File diff suppressed because it is too large Load Diff
qa/data/eeschema/issue20173/issue20173.kicad_pcb
+1538
View File
File diff suppressed because it is too large Load Diff
qa/data/eeschema/issue20173/issue20173.kicad_pro
+635
View File
@@ -0,0 +1,635 @@
{
"board": {
"3dviewports": [],
"design_settings": {
"defaults": {
"apply_defaults_to_fp_fields": false,
"apply_defaults_to_fp_shapes": false,
"apply_defaults_to_fp_text": false,
"board_outline_line_width": 0.05,
"copper_line_width": 0.2,
"copper_text_italic": false,
"copper_text_size_h": 1.5,
"copper_text_size_v": 1.5,
"copper_text_thickness": 0.3,
"copper_text_upright": false,
"courtyard_line_width": 0.05,
"dimension_precision": 4,
"dimension_units": 3,
"dimensions": {
"arrow_length": 1270000,
"extension_offset": 500000,
"keep_text_aligned": true,
"suppress_zeroes": true,
"text_position": 0,
"units_format": 0
},
"fab_line_width": 0.1,
"fab_text_italic": false,
"fab_text_size_h": 1.0,
"fab_text_size_v": 1.0,
"fab_text_thickness": 0.15,
"fab_text_upright": false,
"other_line_width": 0.1,
"other_text_italic": false,
"other_text_size_h": 1.0,
"other_text_size_v": 1.0,
"other_text_thickness": 0.15,
"other_text_upright": false,
"pads": {
"drill": 0.8,
"height": 1.27,
"width": 2.54
},
"silk_line_width": 0.1,
"silk_text_italic": false,
"silk_text_size_h": 1.0,
"silk_text_size_v": 1.0,
"silk_text_thickness": 0.1,
"silk_text_upright": false,
"zones": {
"min_clearance": 0.5
}
},
"diff_pair_dimensions": [
{
"gap": 0.0,
"via_gap": 0.0,
"width": 0.0
}
],
"drc_exclusions": [],
"meta": {
"version": 2
},
"rule_severities": {
"annular_width": "error",
"clearance": "error",
"connection_width": "warning",
"copper_edge_clearance": "error",
"copper_sliver": "warning",
"courtyards_overlap": "error",
"creepage": "error",
"diff_pair_gap_out_of_range": "error",
"diff_pair_uncoupled_length_too_long": "error",
"drill_out_of_range": "error",
"duplicate_footprints": "warning",
"extra_footprint": "warning",
"footprint": "error",
"footprint_filters_mismatch": "ignore",
"footprint_symbol_mismatch": "warning",
"footprint_type_mismatch": "ignore",
"hole_clearance": "error",
"hole_to_hole": "warning",
"holes_co_located": "warning",
"invalid_outline": "error",
"isolated_copper": "warning",
"item_on_disabled_layer": "error",
"items_not_allowed": "error",
"length_out_of_range": "error",
"lib_footprint_issues": "warning",
"lib_footprint_mismatch": "warning",
"malformed_courtyard": "error",
"microvia_drill_out_of_range": "error",
"mirrored_text_on_front_layer": "warning",
"missing_courtyard": "ignore",
"missing_footprint": "warning",
"net_conflict": "warning",
"nonmirrored_text_on_back_layer": "warning",
"npth_inside_courtyard": "ignore",
"padstack": "warning",
"pth_inside_courtyard": "ignore",
"shorting_items": "error",
"silk_edge_clearance": "warning",
"silk_over_copper": "warning",
"silk_overlap": "warning",
"skew_out_of_range": "error",
"solder_mask_bridge": "error",
"starved_thermal": "error",
"text_height": "warning",
"text_thickness": "warning",
"through_hole_pad_without_hole": "error",
"too_many_vias": "error",
"track_angle": "error",
"track_dangling": "warning",
"track_segment_length": "error",
"track_width": "error",
"tracks_crossing": "error",
"unconnected_items": "error",
"unresolved_variable": "error",
"via_dangling": "warning",
"zones_intersect": "error"
},
"rules": {
"max_error": 0.005,
"min_clearance": 0.0,
"min_connection": 0.0,
"min_copper_edge_clearance": 0.5,
"min_groove_width": 0.0,
"min_hole_clearance": 0.25,
"min_hole_to_hole": 0.25,
"min_microvia_diameter": 0.2,
"min_microvia_drill": 0.1,
"min_resolved_spokes": 2,
"min_silk_clearance": 0.0,
"min_text_height": 0.8,
"min_text_thickness": 0.08,
"min_through_hole_diameter": 0.3,
"min_track_width": 0.0,
"min_via_annular_width": 0.1,
"min_via_diameter": 0.5,
"solder_mask_to_copper_clearance": 0.0,
"use_height_for_length_calcs": true
},
"teardrop_options": [
{
"td_onpthpad": true,
"td_onroundshapesonly": false,
"td_onsmdpad": true,
"td_ontrackend": false,
"td_onvia": true
}
],
"teardrop_parameters": [
{
"td_allow_use_two_tracks": true,
"td_curve_segcount": 0,
"td_height_ratio": 1.0,
"td_length_ratio": 0.5,
"td_maxheight": 2.0,
"td_maxlen": 1.0,
"td_on_pad_in_zone": false,
"td_target_name": "td_round_shape",
"td_width_to_size_filter_ratio": 0.9
},
{
"td_allow_use_two_tracks": true,
"td_curve_segcount": 0,
"td_height_ratio": 1.0,
"td_length_ratio": 0.5,
"td_maxheight": 2.0,
"td_maxlen": 1.0,
"td_on_pad_in_zone": false,
"td_target_name": "td_rect_shape",
"td_width_to_size_filter_ratio": 0.9
},
{
"td_allow_use_two_tracks": true,
"td_curve_segcount": 0,
"td_height_ratio": 1.0,
"td_length_ratio": 0.5,
"td_maxheight": 2.0,
"td_maxlen": 1.0,
"td_on_pad_in_zone": false,
"td_target_name": "td_track_end",
"td_width_to_size_filter_ratio": 0.9
}
],
"track_widths": [
0.0,
0.6
],
"tuning_pattern_settings": {
"diff_pair_defaults": {
"corner_radius_percentage": 80,
"corner_style": 1,
"max_amplitude": 1.0,
"min_amplitude": 0.2,
"single_sided": false,
"spacing": 1.0
},
"diff_pair_skew_defaults": {
"corner_radius_percentage": 80,
"corner_style": 1,
"max_amplitude": 1.0,
"min_amplitude": 0.2,
"single_sided": false,
"spacing": 0.6
},
"single_track_defaults": {
"corner_radius_percentage": 80,
"corner_style": 1,
"max_amplitude": 1.0,
"min_amplitude": 0.2,
"single_sided": false,
"spacing": 0.6
}
},
"via_dimensions": [
{
"diameter": 0.0,
"drill": 0.0
}
],
"zones_allow_external_fillets": false
},
"ipc2581": {
"dist": "",
"distpn": "",
"internal_id": "",
"mfg": "",
"mpn": ""
},
"layer_pairs": [],
"layer_presets": [],
"viewports": []
},
"boards": [],
"cvpcb": {
"equivalence_files": []
},
"erc": {
"erc_exclusions": [],
"meta": {
"version": 0
},
"pin_map": [
[
0,
0,
0,
0,
0,
0,
1,
0,
0,
0,
0,
2
],
[
0,
2,
0,
1,
0,
0,
1,
0,
2,
2,
2,
2
],
[
0,
0,
0,
0,
0,
0,
1,
0,
1,
0,
1,
2
],
[
0,
1,
0,
0,
0,
0,
1,
1,
2,
1,
1,
2
],
[
0,
0,
0,
0,
0,
0,
1,
0,
0,
0,
0,
2
],
[
0,
0,
0,
0,
0,
0,
0,
0,
0,
0,
0,
2
],
[
1,
1,
1,
1,
1,
0,
1,
1,
1,
1,
1,
2
],
[
0,
0,
0,
1,
0,
0,
1,
0,
0,
0,
0,
2
],
[
0,
2,
1,
2,
0,
0,
1,
0,
2,
2,
2,
2
],
[
0,
2,
0,
1,
0,
0,
1,
0,
2,
0,
0,
2
],
[
0,
2,
1,
1,
0,
0,
1,
0,
2,
0,
0,
2
],
[
2,
2,
2,
2,
2,
2,
2,
2,
2,
2,
2,
2
]
],
"rule_severities": {
"bus_definition_conflict": "error",
"bus_entry_needed": "error",
"bus_to_bus_conflict": "error",
"bus_to_net_conflict": "error",
"different_unit_footprint": "error",
"different_unit_net": "error",
"duplicate_reference": "error",
"duplicate_sheet_names": "error",
"endpoint_off_grid": "warning",
"extra_units": "error",
"footprint_filter": "ignore",
"footprint_link_issues": "warning",
"four_way_junction": "ignore",
"global_label_dangling": "warning",
"hier_label_mismatch": "error",
"label_dangling": "error",
"label_multiple_wires": "warning",
"lib_symbol_issues": "warning",
"lib_symbol_mismatch": "ignore",
"missing_bidi_pin": "warning",
"missing_input_pin": "warning",
"missing_power_pin": "error",
"missing_unit": "warning",
"multiple_net_names": "warning",
"net_not_bus_member": "warning",
"no_connect_connected": "warning",
"no_connect_dangling": "warning",
"pin_not_connected": "error",
"pin_not_driven": "error",
"pin_to_pin": "warning",
"power_pin_not_driven": "error",
"same_local_global_label": "warning",
"similar_label_and_power": "warning",
"similar_labels": "warning",
"similar_power": "warning",
"simulation_model_issue": "ignore",
"single_global_label": "ignore",
"unannotated": "error",
"unconnected_wire_endpoint": "warning",
"unit_value_mismatch": "error",
"unresolved_variable": "error",
"wire_dangling": "error"
}
},
"libraries": {
"pinned_footprint_libs": [],
"pinned_symbol_libs": []
},
"meta": {
"filename": "issue20173.kicad_pro",
"version": 3
},
"net_settings": {
"classes": [
{
"bus_width": 12,
"clearance": 0.2,
"diff_pair_gap": 0.25,
"diff_pair_via_gap": 0.25,
"diff_pair_width": 0.2,
"line_style": 0,
"microvia_diameter": 0.3,
"microvia_drill": 0.1,
"name": "Default",
"pcb_color": "rgba(0, 0, 0, 0.000)",
"priority": 2147483647,
"schematic_color": "rgba(0, 0, 0, 0.000)",
"track_width": 0.2,
"via_diameter": 0.6,
"via_drill": 0.3,
"wire_width": 6
}
],
"meta": {
"version": 4
},
"net_colors": null,
"netclass_assignments": null,
"netclass_patterns": []
},
"pcbnew": {
"last_paths": {
"gencad": "",
"idf": "",
"netlist": "",
"plot": "",
"pos_files": "",
"specctra_dsn": "",
"step": "",
"svg": "",
"vrml": ""
},
"page_layout_descr_file": ""
},
"schematic": {
"annotate_start_num": 0,
"bom_export_filename": "${PROJECTNAME}.csv",
"bom_fmt_presets": [],
"bom_fmt_settings": {
"field_delimiter": ",",
"keep_line_breaks": false,
"keep_tabs": false,
"name": "CSV",
"ref_delimiter": ",",
"ref_range_delimiter": "",
"string_delimiter": "\""
},
"bom_presets": [],
"bom_settings": {
"exclude_dnp": false,
"fields_ordered": [
{
"group_by": false,
"label": "Reference",
"name": "Reference",
"show": true
},
{
"group_by": false,
"label": "Qty",
"name": "${QUANTITY}",
"show": true
},
{
"group_by": true,
"label": "Value",
"name": "Value",
"show": true
},
{
"group_by": true,
"label": "DNP",
"name": "${DNP}",
"show": true
},
{
"group_by": true,
"label": "Exclude from BOM",
"name": "${EXCLUDE_FROM_BOM}",
"show": true
},
{
"group_by": true,
"label": "Exclude from Board",
"name": "${EXCLUDE_FROM_BOARD}",
"show": true
},
{
"group_by": true,
"label": "Footprint",
"name": "Footprint",
"show": true
},
{
"group_by": false,
"label": "Datasheet",
"name": "Datasheet",
"show": true
}
],
"filter_string": "",
"group_symbols": true,
"include_excluded_from_bom": true,
"name": "Default Editing",
"sort_asc": true,
"sort_field": "Referenz"
},
"connection_grid_size": 50.0,
"drawing": {
"dashed_lines_dash_length_ratio": 12.0,
"dashed_lines_gap_length_ratio": 3.0,
"default_line_thickness": 6.0,
"default_text_size": 50.0,
"field_names": [],
"intersheets_ref_own_page": false,
"intersheets_ref_prefix": "",
"intersheets_ref_short": false,
"intersheets_ref_show": false,
"intersheets_ref_suffix": "",
"junction_size_choice": 3,
"label_size_ratio": 0.375,
"operating_point_overlay_i_precision": 3,
"operating_point_overlay_i_range": "~A",
"operating_point_overlay_v_precision": 3,
"operating_point_overlay_v_range": "~V",
"overbar_offset_ratio": 1.23,
"pin_symbol_size": 25.0,
"text_offset_ratio": 0.15
},
"legacy_lib_dir": "",
"legacy_lib_list": [],
"meta": {
"version": 1
},
"net_format_name": "",
"page_layout_descr_file": "",
"plot_directory": "",
"space_save_all_events": true,
"spice_current_sheet_as_root": false,
"spice_external_command": "spice \"%I\"",
"spice_model_current_sheet_as_root": true,
"spice_save_all_currents": false,
"spice_save_all_dissipations": false,
"spice_save_all_voltages": false,
"subpart_first_id": 65,
"subpart_id_separator": 0
},
"sheets": [
[
"21e8e9f8-d4f3-4be5-903f-68da096527c8",
"Root"
],
[
"5c8e081e-c918-45c7-9d6e-0c91e6f2b219",
"ch1"
]
],
"text_variables": {}
}
qa/data/eeschema/issue20173/issue20173.kicad_sch
+67
View File
@@ -0,0 +1,67 @@
(kicad_sch
(version 20250114)
(generator "eeschema")
(generator_version "9.0")
(uuid "21e8e9f8-d4f3-4be5-903f-68da096527c8")
(paper "A4")
(lib_symbols)
(text "- look into subsheet CH1 - find symbols R1, R2, R3, R4, D1, D2\n- back to root sheet\n- copy/paste (CTRL+C, CTRL+V) the subsheet to get a second instance of that subsheet\n- look again into subsheet CH1 - still looks good\n- switch to subsheet CH2 - references R5...R8, D3, D4 - looks also good\n- no switch back to original subsheet CH1: suddenly we have there the same references as in subsheet CH2!\n- run ERC: get a ton of \"duplicate references\" errors"
(exclude_from_sim no)
(at 17.78 30.48 0)
(effects
(font
(size 2.54 2.54)
)
(justify left)
)
(uuid "8e1ac652-de72-4718-891f-f676745884c5")
)
(sheet
(at 48.26 49.53)
(size 67.31 25.4)
(exclude_from_sim no)
(in_bom yes)
(on_board yes)
(dnp no)
(fields_autoplaced yes)
(stroke
(width 0.1524)
(type solid)
)
(fill
(color 0 0 0 0.0000)
)
(uuid "5c8e081e-c918-45c7-9d6e-0c91e6f2b219")
(property "Sheetname" "ch1"
(at 48.26 48.8184 0)
(effects
(font
(size 1.27 1.27)
)
(justify left bottom)
)
)
(property "Sheetfile" "Kicad 9 - multi channel test.kicad_sch"
(at 48.26 75.5146 0)
(effects
(font
(size 1.27 1.27)
)
(justify left top)
)
)
(instances
(project "issue20173"
(path "/21e8e9f8-d4f3-4be5-903f-68da096527c8"
(page "2")
)
)
)
)
(sheet_instances
(path "/"
(page "1")
)
)
(embedded_fonts no)
)
qa/tests/eeschema/CMakeLists.txt
+1
View File
@@ -64,6 +64,7 @@ set( QA_EESCHEMA_SRCS
erc/test_erc_wire_bus_entry.cpp
test_connectivity_algo.cpp
test_issue20173_paste_subsheet.cpp
test_nc_pin_connectivity.cpp
test_crossing_label.cpp
test_eagle_plugin.cpp
qa/tests/eeschema/test_issue20173_paste_subsheet.cpp
+710
View File
@@ -0,0 +1,710 @@
/*
* This program source code file is part of KiCad, a free EDA CAD application.
*
* Copyright The KiCad Developers, see AUTHORS.TXT for contributors.
*
* This program is free software; you can redistribute it and/or
* modify it under the terms of the GNU General Public License
* as published by the Free Software Foundation; either version 3
* of the License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with this program; if not, you may find one here:
* https://www.gnu.org/licenses/gpl-3.0.en.html
* or you may search the http://www.gnu.org website for the version 3 license,
* or you may write to the Free Software Foundation, Inc.,
* 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA
*/
/**
* @file test_issue20173_paste_subsheet.cpp
*
* Test for issue #20173: Copy/paste subsheet causes original sheet's references to be corrupted.
*
* The test verifies that when a subsheet is copy/pasted to create a second instance sharing
* the same screen file, the original sheet's symbol references are preserved correctly.
*
* Bug scenario:
* - Start with a root sheet containing subsheet CH1 with symbols R1, R2, R3, R4, D1, D2
* - Copy/paste CH1 to create CH2 (which shares the same screen as CH1)
* - CH2 gets new references: R5...R8, D3, D4 (via annotation)
* - BUG: When navigating back to CH1, references show R5...R8, D3, D4 instead of R1...R4, D1, D2
*
* Root cause: When a shared screen is used, symbol instances for the original path may be
* incorrectly modified or lost during the paste operation.
*/
#include <qa_utils/wx_utils/unit_test_utils.h>
#include <schematic_utils/schematic_file_util.h>
#include <connection_graph.h>
#include <schematic.h>
#include <sch_sheet.h>
#include <sch_screen.h>
#include <sch_symbol.h>
#include <settings/settings_manager.h>
#include <locale_io.h>
#include <project.h>
struct ISSUE20173_FIXTURE
{
ISSUE20173_FIXTURE()
{ }
SETTINGS_MANAGER m_settingsManager;
std::unique_ptr<SCHEMATIC> m_schematic;
};
/**
* Helper function to get all symbol references for a given sheet path.
* Returns a map of symbol UUID -> reference designator.
*/
std::map<KIID, wxString> GetSymbolReferences( const SCH_SHEET_PATH& aPath )
{
std::map<KIID, wxString> refs;
if( !aPath.LastScreen() )
return refs;
for( SCH_ITEM* item : aPath.LastScreen()->Items().OfType( SCH_SYMBOL_T ) )
{
SCH_SYMBOL* symbol = static_cast<SCH_SYMBOL*>( item );
// Skip power symbols (references starting with #)
wxString ref = symbol->GetRef( &aPath );
if( !ref.IsEmpty() && ref[0] != '#' )
refs[symbol->m_Uuid] = ref;
}
return refs;
}
/**
* Test that simulates the paste flow more closely, including instance pruning.
*
* This test specifically checks whether the PruneOrphanedSymbolInstances call
* after paste incorrectly removes the original sheet's instances.
*/
BOOST_FIXTURE_TEST_CASE( Issue20173PruningAfterPaste, ISSUE20173_FIXTURE )
{
LOCALE_IO dummy;
KI_TEST::LoadSchematic( m_settingsManager, "issue20173/issue20173", m_schematic );
SCH_SHEET_LIST sheets = m_schematic->BuildSheetListSortedByPageNumbers();
// Find the ch1 subsheet
SCH_SHEET_PATH ch1Path;
SCH_SHEET* ch1Sheet = nullptr;
for( const SCH_SHEET_PATH& path : sheets )
{
if( path.size() > 1 )
{
ch1Path = path;
ch1Sheet = path.Last();
break;
}
}
BOOST_REQUIRE( ch1Sheet != nullptr );
SCH_SCREEN* sharedScreen = ch1Sheet->GetScreen();
BOOST_REQUIRE( sharedScreen != nullptr );
// Set up proper instances for ch1 (in case the loaded data has stale UUIDs)
std::map<KIID, wxString> originalRefs;
for( SCH_ITEM* item : sharedScreen->Items().OfType( SCH_SYMBOL_T ) )
{
SCH_SYMBOL* symbol = static_cast<SCH_SYMBOL*>( item );
wxString ref = symbol->GetField( REFERENCE_FIELD )->GetText();
if( ref.IsEmpty() || ref[0] == '#' )
continue;
symbol->SetRef( &ch1Path, ref );
originalRefs[symbol->m_Uuid] = ref;
}
// Create ch2 with shared screen (simulating paste)
SCH_SHEET* ch2Sheet = new SCH_SHEET( m_schematic.get() );
ch2Sheet->SetName( "ch2" );
ch2Sheet->SetFileName( ch1Sheet->GetFileName() );
ch2Sheet->SetScreen( sharedScreen );
ch2Sheet->SetPosition( ch1Sheet->GetPosition() + VECTOR2I( schIUScale.MilsToIU( 1000 ), 0 ) );
// Add to root and refresh hierarchy
m_schematic->RootScreen()->Append( ch2Sheet );
m_schematic->RefreshHierarchy();
// Get updated hierarchy
sheets = m_schematic->BuildSheetListSortedByPageNumbers();
SCH_SHEET_PATH ch2Path;
for( const SCH_SHEET_PATH& path : sheets )
{
if( path.size() > 1 && path.Last() == ch2Sheet )
{
ch2Path = path;
break;
}
}
BOOST_REQUIRE( ch2Path.size() > 1 );
// Add ch2 instances with different references
for( SCH_ITEM* item : sharedScreen->Items().OfType( SCH_SYMBOL_T ) )
{
SCH_SYMBOL* symbol = static_cast<SCH_SYMBOL*>( item );
wxString ch1Ref = symbol->GetRef( &ch1Path );
if( ch1Ref.IsEmpty() || ch1Ref[0] == '#' )
continue;
// Create ch2 reference (R1 -> R101)
wxString ch2Ref = ch1Ref;
if( ch2Ref.length() > 1 )
{
wxString prefix = ch2Ref.substr( 0, 1 );
long num = 0;
if( ch2Ref.substr( 1 ).ToLong( &num ) )
ch2Ref = prefix + wxString::Format( "%ld", num + 100 );
}
symbol->SetRef( &ch2Path, ch2Ref );
}
// Verify instances are set up correctly before pruning
BOOST_TEST_MESSAGE( "Before pruning:" );
for( SCH_ITEM* item : sharedScreen->Items().OfType( SCH_SYMBOL_T ) )
{
SCH_SYMBOL* symbol = static_cast<SCH_SYMBOL*>( item );
if( symbol->GetRef( &ch1Path )[0] == '#' )
continue;
BOOST_TEST_MESSAGE( " " << symbol->m_Uuid.AsString() << " ch1=" << symbol->GetRef( &ch1Path )
<< " ch2=" << symbol->GetRef( &ch2Path ) );
break;
}
// Now call the pruning that happens after paste
// This is what SCH_EDITOR_CONTROL::Paste does at line 2615
SCH_SCREENS allScreens( m_schematic->Root() );
allScreens.PruneOrphanedSymbolInstances( m_schematic->Prj().GetProjectName(), sheets );
BOOST_TEST_MESSAGE( "After pruning:" );
// Verify ch1 instances are still intact after pruning
bool ch1InstancesIntact = true;
for( const auto& [uuid, originalRef] : originalRefs )
{
// Find the symbol
SCH_SYMBOL* symbol = nullptr;
for( SCH_ITEM* item : sharedScreen->Items().OfType( SCH_SYMBOL_T ) )
{
if( item->m_Uuid == uuid )
{
symbol = static_cast<SCH_SYMBOL*>( item );
break;
}
}
if( !symbol )
continue;
wxString ch1RefAfter = symbol->GetRef( &ch1Path );
wxString ch2RefAfter = symbol->GetRef( &ch2Path );
BOOST_TEST_MESSAGE( " " << uuid.AsString() << " ch1=" << ch1RefAfter << " ch2="
<< ch2RefAfter << " (original=" << originalRef << ")" );
if( ch1RefAfter != originalRef )
{
BOOST_TEST_MESSAGE( " MISMATCH: ch1 reference changed from " << originalRef << " to "
<< ch1RefAfter );
ch1InstancesIntact = false;
}
}
BOOST_CHECK_MESSAGE( ch1InstancesIntact,
"CH1 instances should be preserved after paste and pruning" );
}
/**
* Test that ClearAnnotation doesn't corrupt REFERENCE field in a way that affects GetRef.
*
* ClearAnnotation modifies both the instance reference AND the REFERENCE field text.
* If GetRef falls back to the field text (when instance doesn't match), this can cause issues.
*/
BOOST_FIXTURE_TEST_CASE( Issue20173ClearAnnotationFieldCorruption, ISSUE20173_FIXTURE )
{
LOCALE_IO dummy;
KI_TEST::LoadSchematic( m_settingsManager, "issue20173/issue20173", m_schematic );
SCH_SHEET_LIST sheets = m_schematic->BuildSheetListSortedByPageNumbers();
// Find the ch1 subsheet
SCH_SHEET_PATH ch1Path;
SCH_SHEET* ch1Sheet = nullptr;
for( const SCH_SHEET_PATH& path : sheets )
{
if( path.size() > 1 )
{
ch1Path = path;
ch1Sheet = path.Last();
break;
}
}
BOOST_REQUIRE( ch1Sheet != nullptr );
SCH_SCREEN* sharedScreen = ch1Sheet->GetScreen();
BOOST_REQUIRE( sharedScreen != nullptr );
// Set up proper instances for ch1 (fix any stale UUID issues from loading)
std::map<KIID, wxString> ch1OriginalRefs;
for( SCH_ITEM* item : sharedScreen->Items().OfType( SCH_SYMBOL_T ) )
{
SCH_SYMBOL* symbol = static_cast<SCH_SYMBOL*>( item );
wxString ref = symbol->GetField( REFERENCE_FIELD )->GetText();
if( ref.IsEmpty() || ref[0] == '#' )
continue;
symbol->SetRef( &ch1Path, ref );
ch1OriginalRefs[symbol->m_Uuid] = ref;
}
// Create ch2 with shared screen
SCH_SHEET* ch2Sheet = new SCH_SHEET( m_schematic.get() );
ch2Sheet->SetName( "ch2" );
ch2Sheet->SetFileName( ch1Sheet->GetFileName() );
ch2Sheet->SetScreen( sharedScreen );
m_schematic->RootScreen()->Append( ch2Sheet );
m_schematic->RefreshHierarchy();
sheets = m_schematic->BuildSheetListSortedByPageNumbers();
SCH_SHEET_PATH ch2Path;
for( const SCH_SHEET_PATH& path : sheets )
{
if( path.size() > 1 && path.Last() == ch2Sheet )
{
ch2Path = path;
break;
}
}
BOOST_REQUIRE( ch2Path.size() > 1 );
// Add ch2 instances and call ClearAnnotation (simulating paste with !forceKeepAnnotations)
for( SCH_ITEM* item : sharedScreen->Items().OfType( SCH_SYMBOL_T ) )
{
SCH_SYMBOL* symbol = static_cast<SCH_SYMBOL*>( item );
wxString ch1Ref = symbol->GetRef( &ch1Path );
if( ch1Ref.IsEmpty() || ch1Ref[0] == '#' )
continue;
// Add instance for ch2 (like updatePastedSymbol does)
symbol->SetRef( &ch2Path, ch1Ref );
// Now call ClearAnnotation for ch2 path (like updatePastedSymbol does when !forceKeepAnnotations)
symbol->ClearAnnotation( &ch2Path, false );
}
// Verify ch1 references are unchanged after ClearAnnotation on ch2
bool ch1RefsPreserved = true;
for( const auto& [uuid, originalRef] : ch1OriginalRefs )
{
SCH_SYMBOL* symbol = nullptr;
for( SCH_ITEM* item : sharedScreen->Items().OfType( SCH_SYMBOL_T ) )
{
if( item->m_Uuid == uuid )
{
symbol = static_cast<SCH_SYMBOL*>( item );
break;
}
}
if( !symbol )
continue;
wxString ch1RefAfter = symbol->GetRef( &ch1Path );
if( ch1RefAfter != originalRef )
{
BOOST_TEST_MESSAGE( "Symbol " << uuid.AsString() << " ch1 reference changed from "
<< originalRef << " to " << ch1RefAfter
<< " after ClearAnnotation on ch2" );
ch1RefsPreserved = false;
}
}
BOOST_CHECK_MESSAGE( ch1RefsPreserved,
"CH1 references should be preserved after ClearAnnotation on CH2" );
}
/**
* Test that reproduces the actual bug: ClearAnnotation corrupts field text.
*
* When ClearAnnotation is called for a specific path (CH2), it incorrectly modifies
* the REFERENCE field text. If GetRef later falls back to field text (due to
* mismatched instance paths), it returns the corrupted value.
*/
BOOST_FIXTURE_TEST_CASE( Issue20173FieldTextCorruption, ISSUE20173_FIXTURE )
{
LOCALE_IO dummy;
KI_TEST::LoadSchematic( m_settingsManager, "issue20173/issue20173", m_schematic );
SCH_SHEET_LIST sheets = m_schematic->BuildSheetListSortedByPageNumbers();
// Find the ch1 subsheet
SCH_SHEET_PATH ch1Path;
SCH_SHEET* ch1Sheet = nullptr;
for( const SCH_SHEET_PATH& path : sheets )
{
if( path.size() > 1 )
{
ch1Path = path;
ch1Sheet = path.Last();
break;
}
}
BOOST_REQUIRE( ch1Sheet != nullptr );
SCH_SCREEN* sharedScreen = ch1Sheet->GetScreen();
BOOST_REQUIRE( sharedScreen != nullptr );
// Capture original field text values (these are what GetRef falls back to)
std::map<KIID, wxString> originalFieldText;
for( SCH_ITEM* item : sharedScreen->Items().OfType( SCH_SYMBOL_T ) )
{
SCH_SYMBOL* symbol = static_cast<SCH_SYMBOL*>( item );
wxString ref = symbol->GetField( REFERENCE_FIELD )->GetText();
if( !ref.IsEmpty() && ref[0] != '#' )
originalFieldText[symbol->m_Uuid] = ref;
}
BOOST_REQUIRE( originalFieldText.size() > 0 );
BOOST_TEST_MESSAGE( "Original field text values:" );
for( const auto& [uuid, ref] : originalFieldText )
BOOST_TEST_MESSAGE( " " << uuid.AsString() << " -> " << ref );
// Create ch2 with shared screen (simulating paste)
SCH_SHEET* ch2Sheet = new SCH_SHEET( m_schematic.get() );
ch2Sheet->SetName( "ch2" );
ch2Sheet->SetFileName( ch1Sheet->GetFileName() );
ch2Sheet->SetScreen( sharedScreen );
m_schematic->RootScreen()->Append( ch2Sheet );
m_schematic->RefreshHierarchy();
sheets = m_schematic->BuildSheetListSortedByPageNumbers();
SCH_SHEET_PATH ch2Path;
for( const SCH_SHEET_PATH& path : sheets )
{
if( path.size() > 1 && path.Last() == ch2Sheet )
{
ch2Path = path;
break;
}
}
BOOST_REQUIRE( ch2Path.size() > 1 );
// Simulate what updatePastedSymbol does:
// 1. Add instance for new path
// 2. Call ClearAnnotation for new path
for( SCH_ITEM* item : sharedScreen->Items().OfType( SCH_SYMBOL_T ) )
{
SCH_SYMBOL* symbol = static_cast<SCH_SYMBOL*>( item );
wxString fieldRef = symbol->GetField( REFERENCE_FIELD )->GetText();
if( fieldRef.IsEmpty() || fieldRef[0] == '#' )
continue;
// Add instance for ch2 (like AddHierarchicalReference does)
symbol->SetRef( &ch2Path, fieldRef );
// Now call ClearAnnotation for ch2 path (like updatePastedSymbol does)
symbol->ClearAnnotation( &ch2Path, false );
}
// Check if field text is corrupted
BOOST_TEST_MESSAGE( "Field text after ClearAnnotation on ch2:" );
bool fieldTextCorrupted = false;
for( const auto& [uuid, originalRef] : originalFieldText )
{
SCH_SYMBOL* symbol = nullptr;
for( SCH_ITEM* item : sharedScreen->Items().OfType( SCH_SYMBOL_T ) )
{
if( item->m_Uuid == uuid )
{
symbol = static_cast<SCH_SYMBOL*>( item );
break;
}
}
if( !symbol )
continue;
wxString currentFieldText = symbol->GetField( REFERENCE_FIELD )->GetText();
BOOST_TEST_MESSAGE( " " << uuid.AsString() << " -> " << currentFieldText
<< " (was " << originalRef << ")" );
if( currentFieldText != originalRef )
{
fieldTextCorrupted = true;
BOOST_TEST_MESSAGE( " CORRUPTED!" );
}
}
// The bug: ClearAnnotation modifies field text even when called for a specific path
// This causes GetRef to return wrong value when falling back to field text
BOOST_CHECK_MESSAGE( !fieldTextCorrupted,
"Field text should NOT be corrupted by ClearAnnotation on different path" );
}
/**
* Test that symbols on a shared screen maintain independent instances for each sheet path.
*
* This tests the fundamental requirement that symbols can have different reference
* designators in different sheet instances, even when sharing the same screen.
*/
BOOST_FIXTURE_TEST_CASE( Issue20173SharedScreenInstances, ISSUE20173_FIXTURE )
{
LOCALE_IO dummy;
KI_TEST::LoadSchematic( m_settingsManager, "issue20173/issue20173", m_schematic );
SCH_SHEET_LIST sheets = m_schematic->BuildSheetListSortedByPageNumbers();
// Find the ch1 subsheet
SCH_SHEET_PATH ch1Path;
SCH_SHEET* ch1Sheet = nullptr;
for( const SCH_SHEET_PATH& path : sheets )
{
if( path.size() > 1 )
{
ch1Path = path;
ch1Sheet = path.Last();
break;
}
}
BOOST_REQUIRE( ch1Sheet != nullptr );
SCH_SCREEN* sharedScreen = ch1Sheet->GetScreen();
BOOST_REQUIRE( sharedScreen != nullptr );
// Get root path for adding new sheet
SCH_SHEET_PATH rootPath;
rootPath.push_back( &m_schematic->Root() );
// Create ch2 with shared screen
SCH_SHEET* ch2Sheet = new SCH_SHEET( m_schematic.get() );
ch2Sheet->SetName( "ch2" );
ch2Sheet->SetFileName( ch1Sheet->GetFileName() );
ch2Sheet->SetScreen( sharedScreen );
ch2Sheet->SetPosition( ch1Sheet->GetPosition() + VECTOR2I( schIUScale.MilsToIU( 1000 ), 0 ) );
// Manually add ch2 to root screen
m_schematic->RootScreen()->Append( ch2Sheet );
m_schematic->RefreshHierarchy();
// Rebuild sheets after adding ch2
sheets = m_schematic->BuildSheetListSortedByPageNumbers();
// Find ch2 path from the hierarchy (don't construct manually)
SCH_SHEET_PATH ch2Path;
for( const SCH_SHEET_PATH& path : sheets )
{
if( path.size() > 1 && path.Last() == ch2Sheet )
{
ch2Path = path;
break;
}
}
BOOST_REQUIRE_MESSAGE( ch2Path.size() > 1, "Should find ch2 in hierarchy" );
// First, ensure all symbols have proper instances for ch1Path
// (The loaded schematic might have stale instance data with old sheet UUIDs)
int symbolIndex = 0;
for( SCH_ITEM* item : sharedScreen->Items().OfType( SCH_SYMBOL_T ) )
{
SCH_SYMBOL* symbol = static_cast<SCH_SYMBOL*>( item );
// Skip power symbols
wxString currentRef = symbol->GetField( REFERENCE_FIELD )->GetText();
if( currentRef.IsEmpty() || currentRef[0] == '#' )
continue;
// Set up proper ch1 reference using the current field text
symbol->SetRef( &ch1Path, currentRef );
symbolIndex++;
}
BOOST_TEST_MESSAGE( "Set up " << symbolIndex << " symbols with ch1 instances" );
// Now set different references for ch2
for( SCH_ITEM* item : sharedScreen->Items().OfType( SCH_SYMBOL_T ) )
{
SCH_SYMBOL* symbol = static_cast<SCH_SYMBOL*>( item );
wxString ch1Ref = symbol->GetRef( &ch1Path );
// Skip power symbols
if( ch1Ref.IsEmpty() || ch1Ref[0] == '#' )
continue;
// Add a different reference for ch2 (R1 -> R101, D1 -> D101, etc.)
wxString ch2Ref = ch1Ref;
if( ch2Ref.length() > 1 )
{
wxString prefix = ch2Ref.substr( 0, 1 );
long num = 0;
if( ch2Ref.substr( 1 ).ToLong( &num ) )
ch2Ref = prefix + wxString::Format( "%ld", num + 100 );
}
symbol->SetRef( &ch2Path, ch2Ref );
// Verify ch1 reference is unchanged
wxString ch1RefAfter = symbol->GetRef( &ch1Path );
BOOST_CHECK_MESSAGE( ch1Ref == ch1RefAfter,
"Setting ch2 reference should not change ch1 reference for symbol "
<< symbol->m_Uuid.AsString() );
// Verify ch2 has its own reference
wxString ch2RefAfter = symbol->GetRef( &ch2Path );
BOOST_CHECK_MESSAGE( ch2Ref == ch2RefAfter,
"ch2 should have its own reference for symbol "
<< symbol->m_Uuid.AsString() );
}
// Debug: Print ch1 and ch2 paths
BOOST_TEST_MESSAGE( "ch1Path: " << ch1Path.Path().AsString() );
BOOST_TEST_MESSAGE( "ch2Path: " << ch2Path.Path().AsString() );
// Debug: Print instances for first non-power symbol before UpdateAllScreenReferences
BOOST_TEST_MESSAGE( "Before UpdateAllScreenReferences:" );
for( SCH_ITEM* item : sharedScreen->Items().OfType( SCH_SYMBOL_T ) )
{
SCH_SYMBOL* symbol = static_cast<SCH_SYMBOL*>( item );
if( symbol->GetRef( &ch1Path )[0] == '#' )
continue;
BOOST_TEST_MESSAGE( "Symbol " << symbol->m_Uuid.AsString() << " instances:" );
for( const SCH_SYMBOL_INSTANCE& inst : symbol->GetInstances() )
{
BOOST_TEST_MESSAGE( " Path: " << inst.m_Path.AsString() << " -> " << inst.m_Reference );
}
BOOST_TEST_MESSAGE( " Field text: " << symbol->GetField( REFERENCE_FIELD )->GetText() );
BOOST_TEST_MESSAGE( " GetRef(ch1): " << symbol->GetRef( &ch1Path ) );
BOOST_TEST_MESSAGE( " GetRef(ch2): " << symbol->GetRef( &ch2Path ) );
break; // Only print first symbol for brevity
}
// Now test UpdateAllScreenReferences doesn't corrupt the other instance
ch2Path.UpdateAllScreenReferences();
BOOST_TEST_MESSAGE( "After ch2Path.UpdateAllScreenReferences():" );
for( SCH_ITEM* item : sharedScreen->Items().OfType( SCH_SYMBOL_T ) )
{
SCH_SYMBOL* symbol = static_cast<SCH_SYMBOL*>( item );
if( symbol->GetRef( &ch1Path )[0] == '#' )
continue;
BOOST_TEST_MESSAGE( " Field text: " << symbol->GetField( REFERENCE_FIELD )->GetText() );
BOOST_TEST_MESSAGE( " GetRef(ch1): " << symbol->GetRef( &ch1Path ) );
BOOST_TEST_MESSAGE( " GetRef(ch2): " << symbol->GetRef( &ch2Path ) );
break;
}
ch1Path.UpdateAllScreenReferences();
BOOST_TEST_MESSAGE( "After ch1Path.UpdateAllScreenReferences():" );
for( SCH_ITEM* item : sharedScreen->Items().OfType( SCH_SYMBOL_T ) )
{
SCH_SYMBOL* symbol = static_cast<SCH_SYMBOL*>( item );
if( symbol->GetRef( &ch1Path )[0] == '#' )
continue;
BOOST_TEST_MESSAGE( " Field text: " << symbol->GetField( REFERENCE_FIELD )->GetText() );
BOOST_TEST_MESSAGE( " GetRef(ch1): " << symbol->GetRef( &ch1Path ) );
BOOST_TEST_MESSAGE( " GetRef(ch2): " << symbol->GetRef( &ch2Path ) );
break;
}
// Verify references are still correct after switching
for( SCH_ITEM* item : sharedScreen->Items().OfType( SCH_SYMBOL_T ) )
{
SCH_SYMBOL* symbol = static_cast<SCH_SYMBOL*>( item );
if( symbol->GetRef( &ch1Path )[0] == '#' )
continue;
// Check that instances exist and are different
wxString ch1Ref = symbol->GetRef( &ch1Path );
wxString ch2Ref = symbol->GetRef( &ch2Path );
// Since we set ch2 = ch1 + 100, they should be different
BOOST_CHECK_MESSAGE( ch1Ref != ch2Ref,
"ch1 and ch2 should have different references after navigation" );
}
}