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

[M4x]

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:

Spreadsheet

Snip macro screenshot-6f9fe3.png (8.88 KiB) Viewed 1474 times

To make it clear where I've put in the Cell Start and Cell End values:

Example 1: Cell Start and Cell End

Snip macro screenshot-e14e16.png (12.41 KiB) Viewed 1474 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:

Results of example 1, 2 and 3

Snip macro screenshot-ddf067.png (16.32 KiB) Viewed 1474 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)

[chrisb]

Confirmed.

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

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)

[aapo]

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.

[M4x]

Sounds good, I'm looking forward to it!

[aapo]

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!

20201120_FreeCAD_TechDraw_spreadsheet_bug_fixed.png (69.29 KiB) Viewed 1311 times

[chennes]

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.

[aapo]

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.

[M4x]

Thank you very much!

[aapo]

wmayer, if you have a tiny bit of time left from the FC core development, I'd like to disturb you with this simple bug fix waiting to be merged, please! It is PR #4068 https://github.com/FreeCAD/FreeCAD/pull/4068 and fixes a TechDraw spreadsheet rendering bug, and chennes has already reviewed it in GitHub. Thanks!

pinged by pinger macro

[aapo]

This has now been merged to FreeCAD master, and the bug should be fixed in the binaries built in the next build cycle. Thanks!