Closed gillins closed 9 months ago
Very industrious of you.
I was not at all keen on the "just the selected rows" idea, for all the sorts of reasons you mention. So, I think this sounds like a much better approach. I don't (yet?) understand the code, but judging from the description in the post you referenced, it sounds like it should be a much more general solution. All of which assumes it will actually work, but fingers crossed.
Well done @gillins
Hey guys, I think this is ready for an initial test. It ended up being quite complex doing the custom scrollbar thing - Qt always wanted to do something different from me, but I think it's close. All the various combinations of clicking to select, Ctrl+Click, Shift+Click, geographic select, select by expression was quite a lot of work to handle. Probably should have a testing framework here, but that would be a lot of work now.
Let me know how you go.
Well done. This seems to work fine for me.
I did a little bit of side-by-side comparison with the current version, and the new one seems noticeably faster when selecting on the map. Very quick, now.
The only thing I noticed was that selecting a row in the table prints command 0x23
on stdout. Presumably a left-over debug statement.
Very impressive.
Thanks Neil, testing much appreciated. Did you try a RAT with more than 100million rows? Yep debug statement still in there, trying to work out when when you ctrl+click on a second row the first one gets unselected. Do you see this too? Shift+click on a second row to select everything in between seemed OK.
Oh, I see. Had not noticed, but yes, you are right, there is something weird with ctrl+click. If there is already a range (using shift+click), then ctrl+click will correctly add a separate row to that range. However, if there is only a single row selected, then ctrl+click on a separate row will de-select the original row. Not quite right.
re: row count. No, I was using that 64 million row one we used from before. I am sure you are surprised to know that I don't have a >100 million row file handy :-), and I confess did not have the energy to generate one. Let me know if you would like me to.
There was also something weird with Shift+Click if you selected a row down the bottom of the visible window, scrolled down then selected a row near the top it all got very confused...
So, I've re done the implementation to handle the selection ourselves rather than relying on Qt at all for this. The Qt thing had a few problems for our case anyway so already had horrible workarounds so I think this is cleaner anyway. Seems to work ok for everything I've tried but would appreciate a double check.
re: row count. I just did a sneaky and called rat.SetRowCount(really_large_number)
when the dataset is open in update mode.
I just had a quick whirl. Seems to be some problems. I tried a select on the map, and it highlights the selected row in the table, but then limits the scrolling so it won't scroll before that row.
The shift+click and crtl+click stuff now works better though. :-)
Hmmm, I think it's anytime that you move the scroll bar then try and scroll wheel back up?
No, this is without touching the scroll bar, just scroll wheel. Start fresh, zoom in to see a single segment, open query tool, click segment, try scrolling table.
If I then try to use the scroll bar, that works, and then the scroll wheel will go as far as the scroll bar took it, but no further.
I also managed to make it narrow the mouse focus for the scroll wheel so it only scrolled while the mouse pointer was within the bounds of the scroll bar panel, although I have not reproduced that again, so not sure what I did.
OK try it now...
No, scroll wheel does not work at all, now.
Are you sure? Do you have your cursor over the table when you use the wheel?
Yes.
However, that made me try moving it, and the scroll wheel works if the mouse cursor is over the narrow panel of the scroll bar, but not outside that.
Hmm, that's slightly worrying. querywindow.py
line 640, can you confirm that wheelEvent
isn't being called at all?
It is being called, but the initial value of dy is zero, so it never goes into that if block.
That's so weird, is fine on my machine. What is dx
??
I mean e.pixelDelta().x()
Also zero
Hmm the documentation is recommending e.angleDelta().y() instead. pixelDelta "Unreliable on X11". Is that a sensible value on your system?
e.angleDelta.y() is either 120 or -120, depending on which way I scroll. I changed it to use that for dy, and the scroll wheel now works fine.
Well done :-)
Pulled your last commit, that works fine.
Thanks for that. I'm sure there will be other gremlins that are OS specific, but hard to test everything!
One other thing: when you have a file with a small RAT (see below for creation script). It limits the number of rows shown, but you can scroll down right past the data (hard to explain). Is this a problem?
#!/usr/bin/env python
import numpy
from osgeo import gdal
gdal.UseExceptions()
driver = gdal.GetDriverByName('KEA')
ds = driver.Create('small.kea', 100, 100, 1, gdal.GDT_Byte)
layer = ds.GetRasterBand(1)
data = numpy.random.randint(0, 15, size=(100, 100))
layer.WriteArray(data)
layer.SetMetadataItem('LAYER_TYPE', 'thematic')
rat = layer.GetDefaultRAT()
rat.CreateColumn('Bob', gdal.GFT_Integer, gdal.GFU_Generic)
data = numpy.random.randint(20, 30, size=(15,))
rat.SetRowCount(15)
rat.WriteArray(data, 0)
I see what you mean (thanks for the test code :-). Yes, a bit odd, but no, I don't think that is a big problem. If you can't make that go away, I would not let it stop you from this approach.
Should be fixed now...
Yup, that works for me. :-)
As suggested on this post this PR involves capping the size of the
QTableView
to 10000 elements. For RAT's that are larger than this I translate the table row to a rat row so everything can be scrolled through.There is currently a weird problem where hitting the arrow on the vertical scrollbar scrolls more rows than you would expect... I think I can deal with this using a custom
QScrollBar
implementation.What do you think @neilflood @petescarth ? Is this a good way forward? I know we initially discussed just showing the selected data as a workaround, but we'd still need to deal with what happens when the user selects more than 100million rows...