visit-dav / visit

VisIt - Visualization and Data Analysis for Mesh-based Scientific Data
https://visit.llnl.gov
BSD 3-Clause "New" or "Revised" License
438 stars 116 forks source link

Indexing bug for Pick with file format readers that set cell numbers #1280

Open aowen87 opened 5 years ago

aowen87 commented 5 years ago

The code in avtLocateCellQuery has an incorrect assumption. It assumes that the value of "avtOriginalCellNumbers" for a given cell is the index of that cell in the original data set. So, if zone Z has "avtOriginalCellNumbers" value C, then it is assuming that zone Z was C'th entry in the vtkDataSet returned by the file format reader. This property is true when we auto-generate the avtOriginalCellNumbers array, which is what happens 99% of the time. The 1% comes because we allow file format readers to populate their own "avtOriginalCellNumbers". For example, a reader may decide that the user wants the cell number for the first entry in the vtkDataSet to be reported as "10", not "0". It doesn't happen often, but it could. This becomes a problem with the logic in avtLocateCellQuery. It overwrites the cell index with the avtOriginalCellNumber value: if (canUseCells && origCells) { int comp = origCells->GetNumberOfComponents() 1; foundElement = (int) origCells>GetComponent(foundCell, comp); } It later uses foundElement as an index into the vtkDataSet, because it sets pickAtts.SetElementNumber with foundElement and then later sets "pickedZone" as pickAtts.GetElementNumber. GetZoneCoords(ds, pickedZone); success = RetrieveNodes(ds, pickedZone); When we are retrieving values for secondary variable, this could lead to reading past the end of an array or reading the wrong data. Again, the issue is with file format readers that set up their own array, since their values may not correspond to the index of the cell in the original vtkDataSet.

-----------------------REDMINE MIGRATION----------------------- This ticket was migrated from Redmine. As such, not all information was able to be captured in the transition. Below is a complete record of the original redmine ticket.

Ticket number: 1272 Status: Pending Project: VisIt Tracker: Bug Priority: Normal Subject: Indexing bug for Pick with file format readers that set cell numbers Assigned to: - Category: - Target version: - Author: Hank Childs Start: 12/11/2012 Due date: % Done: 0% Estimated time: Created: 12/11/2012 07:07 pm Updated: 12/18/2012 07:03 pm Likelihood: 2 - Rare Severity: 4 - Crash / Wrong Results Found in version: 2.6.0 Impact: Expected Use: OS: All Support Group: Any Description: The code in avtLocateCellQuery has an incorrect assumption. It assumes that the value of "avtOriginalCellNumbers" for a given cell is the index of that cell in the original data set. So, if zone Z has "avtOriginalCellNumbers" value C, then it is assuming that zone Z was C'th entry in the vtkDataSet returned by the file format reader. This property is true when we auto-generate the avtOriginalCellNumbers array, which is what happens 99% of the time. The 1% comes because we allow file format readers to populate their own "avtOriginalCellNumbers". For example, a reader may decide that the user wants the cell number for the first entry in the vtkDataSet to be reported as "10", not "0". It doesn't happen often, but it could. This becomes a problem with the logic in avtLocateCellQuery. It overwrites the cell index with the avtOriginalCellNumber value: if (canUseCells && origCells) { int comp = origCells->GetNumberOfComponents() 1; foundElement = (int) origCells>GetComponent(foundCell, comp); } It later uses foundElement as an index into the vtkDataSet, because it sets pickAtts.SetElementNumber with foundElement and then later sets "pickedZone" as pickAtts.GetElementNumber. GetZoneCoords(ds, pickedZone); success = RetrieveNodes(ds, pickedZone); When we are retrieving values for secondary variable, this could lead to reading past the end of an array or reading the wrong data. Again, the issue is with file format readers that set up their own array, since their values may not correspond to the index of the cell in the original vtkDataSet.

Comments: I don't think the problem lies in Pick's use of avtOriginalCellNumbers. The origination of this array was exactly so that Pick would know how to index into the vtkDataset served up by the reader. Especially after such things as Slice, where the cell numbers pick first looks at do not correspond to the original data.

griffin28 commented 4 years ago

@aowen87 this sounds like something that you've resolved?

aowen87 commented 4 years ago

I think this is still an issue, but there might be logic in there to handle this now. I do know that the pick query still does use the "cell start" provided by the database reader. Whether or not it's smart enough to revert that offset later on is not something I'm sure of...