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

VID area blocked and slow on big text #5506

Closed
GiuseppeChillemi opened this issue May 24, 2024 · 8 comments
Closed

VID area blocked and slow on big text #5506

GiuseppeChillemi opened this issue May 24, 2024 · 8 comments
Assignees
Labels
GUI status.built A change in codebase has been done to address the ticket. status.tested The change in code has been manually tested and verified to fix the issue.
Milestone

Comments

@GiuseppeChillemi
Copy link

GiuseppeChillemi commented May 24, 2024

Vid Areas, with big text are initialy locked.
Also there is a great slowness in insertion and deletion operations

Gitter discussion

To reproduce
Run the following code
To unlock the area, simply insert a character with CTRL + V

text: "abcdefghijklmnopqrstuvzABCDEFGHIJKLMNOPQRSTUVZ1234567890!"


size: 2048 * 1048 / (length? text)

area-text: copy ""



repeat idx size [

	text: random text
	append area-text rejoin [text lf]
	 
] 


view [
	area area-text 500x500
]

Platform version

-----------RED & PLATFORM VERSION----------- 
RED: [ branch: "master" tag: #v0.6.5 ahead: 29 date: 28-Feb-2024/21:14:05 commit: #186d215db4d9260885838ca4d846c6d5ceb7e005 ]
PLATFORM: [ name: "Windows 11" OS: 'Windows arch: 'x86-64 version: 10.0.0 build: 22631 ]
--------------------------------------------
@dockimbel dockimbel added status.reviewed Ticket has been reviewed and accepted for further processing. GUI labels May 24, 2024
qtxie added a commit that referenced this issue May 24, 2024
@qtxie
Copy link
Contributor

qtxie commented May 24, 2024

Fixed the lock issue. I don't think we can fix the slowness.

@GiuseppeChillemi
Copy link
Author

Fixed the lock issue. I don't think we can fix the slowness.

Hope you can do something as I have actually reached the ceiling. The documents I am working on are more than 1MB and the area is quite unusable.

@dockimbel
Copy link
Member

dockimbel commented May 24, 2024

@qtxie Commenting this scrollbar updating fixes the slowness issue. We need a better way to update the scrollbars in such case. Maybe only when a new row is created/deleted? Or better when the scrollbar receives the focus?

There's also an excessive GC triggering caused by reactivity handling on that same event. It does not seem to affect UI performance, but it's still annoying to see a GC pass on almost every key stroke inside such overloaded area widget...

@dockimbel
Copy link
Member

dockimbel commented May 24, 2024

Actually, the area seems to be fine without that update-scrollbars code except for switching on/off the scrollbars automatically, that is then lost. It should be possible to have a lightweight solution for such need in the area case... Maybe have it set by the user as a face flag?

dockimbel added a commit that referenced this issue May 24, 2024
Temporary fix until object! gets a 'on-get event.
Adds a `no-sync` flag to disable auto-sync of `/text` facet.
Use `system/view/platform/update-text <face>` to force updating of `/text` facet with the GUI widget text content.
@dockimbel
Copy link
Member

Pushed a fix in this branch.

This is a proposed temporary fix until object! gets an 'on-get event (which would allow updating the facet in a lazy way). @qtxie Please review it and let me know your opinion.

  • Removes the slow call to update-scrollbars (auto-activation of scrollbars disabled, we need a solution for that).
  • Adds a new no-sync flag to disable auto-sync of /text facet.
  • Use system/view/platform/update-text <face> to force updating of /text facet with the GUI widget text content.

I've tested it successfully with the above test code with following additions:

view [
    z: area area-text 500x500 with [flags: 'no-sync]
    button "Sync" [system/view/platform/update-text z]
]

@qtxie
Copy link
Contributor

qtxie commented May 25, 2024

@dockimbel I think it's OK to do the changes.

@GiuseppeChillemi
Copy link
Author

Here it works, the slowness has disappeared!

dockimbel added a commit that referenced this issue May 25, 2024
dockimbel added a commit that referenced this issue May 25, 2024
Temporary fix until object! gets a 'on-get event.
Adds a `no-sync` flag to disable auto-sync of `/text` facet.
Use `system/view/platform/update-text <face>` to force updating of `/text` facet with the GUI widget text content.
@dockimbel
Copy link
Member

Fix merged to master. Let us know if any regression on area behavior occurs on Windows platform.

@dockimbel dockimbel added status.built A change in codebase has been done to address the ticket. status.tested The change in code has been manually tested and verified to fix the issue. and removed status.reviewed Ticket has been reviewed and accepted for further processing. labels May 25, 2024
@dockimbel dockimbel added this to the 0.6.6 milestone May 25, 2024
qtxie added a commit that referenced this issue May 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
GUI status.built A change in codebase has been done to address the ticket. status.tested The change in code has been manually tested and verified to fix the issue.
Projects
None yet
Development

No branches or pull requests

3 participants