rojo-rbx / rojo

Rojo enables Roblox developers to use professional-grade software engineering tools
https://rojo.space
Mozilla Public License 2.0
969 stars 182 forks source link

The plugin's diff view sometimes displays incorrectly #968

Open Hexcede opened 2 months ago

Hexcede commented 2 months ago

Occasionally the plugin's diff view displays scripts with a lot of text overlapping, and the line height in the diff view seems to become really small.

image

There is an uncolored layer of text underneath, and a colored layer of text above, and the syntax colored layer of text is the one that suffers from this issue while the layer underneath appears to be in the correct location.

I am not sure what exactly causes this issue and I see it infrequently, but I have a hunch that it may be related to the size of the script. It happens fully consistently on the same large scripts when syncing, persists over restarts of Studio, etc so its clear that the issue is dependent on the content of the script file, likely the length since that's the only notable difference between scripts that display incorrectly and scripts that don't.

This issue occurs on the latest version of the plugin but isn't new and has happened to me once or twice in the past.

kennethloeffler commented 1 week ago

This issue is consistently reproducible with very large scripts - I can get it to happen every time with a very large script. I'll attach a project that it happens with in a moment!

kennethloeffler commented 1 week ago

This issue is reproducible with the following project: massive-script.zip

boatbomber commented 1 week ago

Thanks for the repro! It appears that TextLabel.TextBounds is just straight up lying to us. Fun! I've reported this here: https://devforum.roblox.com/t/textbounds-is-completely-incorrect-for-text-with-many-lines/3252070

Your script has 13,262 lines. TextLabel.TextBounds (and TextService:GetTextBoundsAsync) both give a height of 17,488px. That would mean each line is 1.3 pixels tall, which is why the syntax highlights are so cramped together.

boatbomber commented 1 week ago

Even when I compute the text bounds manually, it's broken because UDim2 Scale doesn't have the precision for the tiny values needed. If I use Offset, it won't work properly on High DPI screens... This sucks.

boatbomber commented 1 week ago

I believe there are more high-dpi users than massive script authors, so I'm inclined to say that we keep this as the lesser of two evils.

boatbomber commented 1 week ago

I actually just thought of a half-decent solution to this that'll also improve performance- I can make the diff view into a virtualized list and highlight each line individually, so that there's fewer labels and no textbounds issue

kennethloeffler commented 1 week ago

Sounds okay to me, but would it work for syntactical constructs that span multiple lines? For example multi-line comments:

--[[
    This is a multi-line doc comment.

    Say hi!
]]
local function sayHi()
end

or multi-line strings:

local s = [[
This 
is 
a 
multi-line 
string
]]
boatbomber commented 1 week ago

The highlights can be computed from the full string, but they'll be rendered line by line in a virtualized list. I didn't explain that well, so thanks for calling it out!