Eitan Traurig (EitanSomething) retitled this revision from Range filters for spreadsheet editor TODO: -Add better default. Default set the Minimum and Maximum to the Minimum and Maximum of the column. to WIIP: Range filters for spreadsheet editor.Jul 20 2021, 11:27 PM

Eitan Traurig (EitanSomething) edited the summary of this revision. (Show Details)

Eitan Traurig (EitanSomething) retitled this revision from WIIP: Range filters for spreadsheet editor to WIP: Range filters for spreadsheet editor.

Eitan Traurig (EitanSomething) edited the summary of this revision. (Show Details)

Eitan Traurig (EitanSomething) edited the summary of this revision. (Show Details)Jul 20 2021, 11:29 PM

Eitan Traurig (EitanSomething) added a project: Geometry Nodes.

Clean up spacing

Harbormaster completed remote builds in B15930: Diff 39761.Jul 20 2021, 11:31 PM

Eitan Traurig (EitanSomething) added a task: T89273: "Range" filter option for spreadsheet row filters.Jul 20 2021, 11:32 PM

Eitan Traurig (EitanSomething) retitled this revision from WIP: Range filters for spreadsheet editor to WIP: Range filter for spreadsheet editor.Jul 21 2021, 2:24 AM

While still having WIP in the title better to also keep as "Changes Planned"

Plenty of problems:

For integers it is changing both min and max together.
The header itself never updates
It doesn't include both Min and Max values in the header
(would be nice to make sure min is always less than max)

Not unique to this, but when using vectors I think we should NOT even bother showing the value in the header:

Hide value in header
Fix integers changing both min and max together.

Harbormaster completed remote builds in B15972: Diff 39841.Jul 23 2021, 1:50 AM

Eitan Traurig (EitanSomething) added inline comments.Jul 23 2021, 1:50 AM

source/blender/makesrna/intern/rna_space.c
7543	Change to `max_int`

Eitan Traurig (EitanSomething) edited the summary of this revision. (Show Details)Jul 23 2021, 1:50 AM

Eitan Traurig (EitanSomething) retitled this revision from WIP: Range filter for spreadsheet editor to Range filter for spreadsheet editor.

Charlie Jolly (charlie) mentioned this in T86749: Geometry Node Community Additions.Jul 23 2021, 10:43 AM

Much better, thanks.

See how we do frame_start and frame_end, for range I think it makes sense to have one pushing the other.
You remove the range from the header altogether, was this intentional? My remark was mostly about vectors.

In D11980#309802, @Dalai Felinto (dfelinto) wrote:

You remove the range from the header altogether, was this intentional? My remark was mostly about vectors.

I removed it because the code is not built to show two values. This is how it should look (1.0,1.0,1.0) >= position >= (-1.0,-1.0,-1.0) . It also won’t be totally visible most if not all of the time

Make sure max never goes below min

Harbormaster completed remote builds in B16000: Diff 39888.Jul 23 2021, 7:36 PM

Hans Goudey (HooglyBoogly) requested changes to this revision.Aug 9 2021, 6:53 AM

Hans Goudey (HooglyBoogly) added a subscriber: Hans Goudey (HooglyBoogly).

Hans Goudey (HooglyBoogly) added inline comments.

source/blender/editors/space_spreadsheet/spreadsheet_row_filter.cc
103	This seems like something that should be done in RNA, not here. Either that or right below, with `std::min` and `std::max`. Best not to remove const unless you really need to
150–154	Same here
200–207	And here
255–265	And here
379	And don't remove const here
source/blender/makesdna/DNA_space_types.h
1895–1902	With all of these types, the row filters are starting to get quite large. Maybe full-on polymorphism is a bit much, but a union could work. It's all a bit unfortunate honestly, maybe there is a better way to handle this.