Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mechanism to deal with very large RATS #65

Merged
merged 13 commits into from
Oct 31, 2023
Merged

Conversation

gillins
Copy link
Member

@gillins gillins commented Oct 6, 2023

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...

@neilflood
Copy link
Member

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

@gillins
Copy link
Member Author

gillins commented Oct 27, 2023

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.

@neilflood
Copy link
Member

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.

@gillins
Copy link
Member Author

gillins commented Oct 27, 2023

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.

@neilflood
Copy link
Member

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.

@gillins
Copy link
Member Author

gillins commented Oct 30, 2023

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.

@gillins gillins marked this pull request as ready for review October 30, 2023 06:04
@neilflood
Copy link
Member

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. :-)

@gillins
Copy link
Member Author

gillins commented Oct 30, 2023

Hmmm, I think it's anytime that you move the scroll bar then try and scroll wheel back up?

@neilflood
Copy link
Member

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.

@gillins
Copy link
Member Author

gillins commented Oct 30, 2023

OK try it now...

@neilflood
Copy link
Member

No, scroll wheel does not work at all, now.

@gillins
Copy link
Member Author

gillins commented Oct 30, 2023

Are you sure? Do you have your cursor over the table when you use the wheel?

@neilflood
Copy link
Member

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.

@gillins
Copy link
Member Author

gillins commented Oct 30, 2023

Hmm, that's slightly worrying. querywindow.py line 640, can you confirm that wheelEvent isn't being called at all?

@neilflood
Copy link
Member

It is being called, but the initial value of dy is zero, so it never goes into that if block.

@gillins
Copy link
Member Author

gillins commented Oct 30, 2023

That's so weird, is fine on my machine. What is dx??

@gillins
Copy link
Member Author

gillins commented Oct 30, 2023

I mean e.pixelDelta().x()

@neilflood
Copy link
Member

Also zero

@gillins
Copy link
Member Author

gillins commented Oct 30, 2023

Hmm the documentation is recommending e.angleDelta().y() instead. pixelDelta "Unreliable on X11". Is that a sensible value on your system?

@neilflood
Copy link
Member

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 :-)

@neilflood
Copy link
Member

Pulled your last commit, that works fine.

@gillins
Copy link
Member Author

gillins commented Oct 30, 2023

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)

@neilflood
Copy link
Member

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.

@gillins
Copy link
Member Author

gillins commented Oct 30, 2023

Should be fixed now...

@neilflood
Copy link
Member

Yup, that works for me. :-)

@gillins gillins changed the title initial implementation of mechanism to deal with very large RATS Mechanism to deal with very large RATS Oct 31, 2023
@gillins gillins merged commit b2f255d into ubarsc:master Oct 31, 2023
2 checks passed
@gillins gillins deleted the big_rat_table branch October 31, 2023 05:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants