[Issue] Spreadsheet View Cell Start and Cell End not always working correctly if column name has more than one letter

Discussions about the development of the TechDraw workbench
User avatar
M4x
Posts: 281
Joined: Sat Mar 11, 2017 9:23 am
Location: Germany

[Issue] Spreadsheet View Cell Start and Cell End not always working correctly if column name has more than one letter

Postby M4x » Mon Nov 09, 2020 9:30 am

Hey everybody,

I think I've discovered an issue regarding the TechDraw workbench and the TechDraw_SpreadsheetView. I can define "Cell Start" and "Cell End" to define which area of a spreadsheet should be displayed withing the SpreadsheetView. I assume, that TechDraw is sorting according to the first letter of the column name. If the first letter is identical, the second letter is taken into account. Disclaimer: I haven't looked into the code. What I describe here are my assumptions based on the behavior I've observed.

This is what the Spreadsheet looks like:
Snip macro screenshot-6f9fe3.png
Spreadsheet
Snip macro screenshot-6f9fe3.png (8.88 KiB) Viewed 479 times

To make it clear where I've put in the Cell Start and Cell End values:
Snip macro screenshot-e14e16.png
Example 1: Cell Start and Cell End
Snip macro screenshot-e14e16.png (12.41 KiB) Viewed 479 times


Example 1
  • Cell Start = X1
  • Cell End = Z3
X comes before Z, everything is fine.

Example 2
  • Cell Start = X1
  • Cell End = AB3
A comes after Z, TechDraw is complaining and won't update the SpreadsheetView.
Sheet001 - cell range is illogical
This is a faulty behaviour, because AB comes after Z.

Example 3
  • Cell Start = AB1
  • Cell End = AD3
The first letter is an A in both cases, no way to sort something already. The second letters are B and D, B comes before D, everything is fine.

The results look like this:
Snip macro screenshot-ddf067.png
Results of example 1, 2 and 3
Snip macro screenshot-ddf067.png (16.32 KiB) Viewed 479 times
Ive made a simple example file. Please confirm this behavior. I'll open a ticket at the tracker if that's okay and the bug is confirmed. If someone would check the code, I'd be very glad too!

OS: Ubuntu 20.04.1 LTS (ubuntu:GNOME/ubuntu)
Word size of OS: 64-bit
Word size of FreeCAD: 64-bit
Version: 0.19.22960 (Git) AppImage
Build type: Release
Branch: master
Hash: c5a4b01d2e4218bcc0eb6650337650a6c65ef0e4
Python version: 3.8.6
Qt version: 5.12.5
Coin version: 4.0.0
OCC version: 7.4.0
Locale: English/United States (en_US)
Attachments
TD_SpreadsheetView_UpdateIssue_v1.FCStd
(16.52 KiB) Downloaded 14 times
Snip macro screenshot-132d31.png
Tree of example file
Snip macro screenshot-132d31.png (12.38 KiB) Viewed 479 times
chrisb
Posts: 29041
Joined: Tue Mar 17, 2015 9:14 am

Re: [Issue] Spreadsheet View Cell Start and Cell End not always working correctly if column name has more than one lette

Postby chrisb » Tue Nov 10, 2020 10:43 pm

Confirmed.

I tried to fool the program by entering CellStart=AA3 and CellEnd=Z3. The error vanishes :) , but the view is empty :cry: .

OS: macOS 10.15
Word size of OS: 64-bit
Word size of FreeCAD: 64-bit
Version: 0.19.22846 (Git)
Build type: Release
Branch: master
Hash: 1f46b72491a0008384a6db4f2615a656249f6f08
Python version: 3.8.6
Qt version: 5.12.5
Coin version: 4.0.0
OCC version: 7.4.0
Locale: C/Default (C)
A Sketcher Lecture with in-depth information is available in English, auf deutsch, en français, en español.
aapo
Posts: 210
Joined: Mon Oct 29, 2018 6:41 pm

Re: [Issue] Spreadsheet View Cell Start and Cell End not always working correctly if column name has more than one lette

Postby aapo » Fri Nov 20, 2020 4:29 pm

M4x wrote:
Mon Nov 09, 2020 9:30 am

Ive made a simple example file. Please confirm this behavior. I'll open a ticket at the tracker if that's okay and the bug is confirmed. If someone would check the code, I'd be very glad too!
Confirmed. There seems to be a relatively simple logical error in a rather long function

Code: Select all

std::string DrawViewSpreadsheet::getSheetImage(void)
{
...
}
in file src/Mod/TechDraw/App/DrawViewSpreadsheet.cpp. Namely, the order of the columns is checked by string comparison, which is alphabetical order; instead of comparing the numerical column indices, which would be conveniently calculated a few lines down in the same function. I don't think there would be need for a ticket, as fixing this bug should be quite simple. I'll try to make a pull request after I've compiled and tested the patch I just made with your test file.
User avatar
M4x
Posts: 281
Joined: Sat Mar 11, 2017 9:23 am
Location: Germany

Re: [Issue] Spreadsheet View Cell Start and Cell End not always working correctly if column name has more than one lette

Postby M4x » Fri Nov 20, 2020 5:23 pm

Sounds good, I'm looking forward to it!
aapo
Posts: 210
Joined: Mon Oct 29, 2018 6:41 pm

Re: [Issue] Spreadsheet View Cell Start and Cell End not always working correctly if column name has more than one lette

Postby aapo » Fri Nov 20, 2020 8:29 pm

M4x wrote:
Fri Nov 20, 2020 5:23 pm
Sounds good, I'm looking forward to it!
It was slightly more complicated than I thought, due to a missing redraw logic in TechDraw spreadsheets, but here it is: Pull request #4068: https://github.com/FreeCAD/FreeCAD/pull/4068. Works for me, but anyone capable of building your own copy of FreeCAD from GitHub sources, please test! :D

20201120_FreeCAD_TechDraw_spreadsheet_bug_fixed.png
20201120_FreeCAD_TechDraw_spreadsheet_bug_fixed.png (69.29 KiB) Viewed 316 times
chennes
Posts: 48
Joined: Fri Dec 23, 2016 3:38 pm
Contact:

Re: [Issue] Spreadsheet View Cell Start and Cell End not always working correctly if column name has more than one lette

Postby chennes » Fri Nov 20, 2020 11:48 pm

Works for me -- at least, it fixes the issue reported here. I see that there is a second problem that it addresses: is there a way for me to test that? I don't quite understand the problem.
Chris Hennes
Pioneer Library System
aapo
Posts: 210
Joined: Mon Oct 29, 2018 6:41 pm

Re: [Issue] Spreadsheet View Cell Start and Cell End not always working correctly if column name has more than one lette

Postby aapo » Sat Nov 21, 2020 7:59 am

chennes wrote:
Fri Nov 20, 2020 11:48 pm
Works for me -- at least, it fixes the issue reported here. I see that there is a second problem that it addresses: is there a way for me to test that? I don't quite understand the problem.
Thanks for testing! :)

When I tried the first fix, and loaded the example file; I noticed that the spreadsheet was not updated, and it was not updated even if an explicit refresh command was clicked in the TechDraw user interface. However, the fix appeared to work after the spreadsheet was edited, which triggered the update mechanism. Digging further in the code, I noticed that the spreadsheets are not affected by the refresh-mechanism, and there is even a comment in the code

Code: Select all

"//should really be called "updateMostViews".  can still be problems to due execution order."
Since the spreadsheet view does not depend on the other view types, I added spreadsheet refresh logic after other refreshes, and this should not cause any execution order problems, as they seem to be kind of independent structures (not the logical spreadsheets, but the TD graphs of them). The easiest way to test the 2nd commit is to load the example file without the 2nd commit, but with the 1st commit applied: Nothing happens to the problematic spreadsheet when the file is loaded, explicit TD refresh does not work for the spreadsheet, and the spreadsheet problem seems to not be fixed, until the user actually edits some fields of the problematic spreadsheet, and it is magically fixed. The 2nd commit addresses this refresh problem.