Better way to register where to place pixels when mouse down

I am creating a pixel art game with GUIs, and so far it’s going good!

However, I am using a while loop to detect if the mouse is currently down. This means that if the player moves the mouse too fast then it’ll skip over areas of the GUI. (as seen here: https://gyazo.com/1b21815421f785b4fc1442f90035d7ec)

How can I register drawing better than a while loop so that it won’t skip over areas of the drawing?

Loop:

while wait() do
    if hold and not game:GetService("Players").LocalPlayer.PlayerGui.ScreenGui.ColorPicker.Visible then
		game:GetService("ReplicatedStorage").Draw:FireServer(mouse.X, mouse.Y)
	end
end

Draw is an event that places the pixel. The “hold” variable is true whenever the mouse is pressed down. The colorpicker thing is just to make sure player’s can’t draw with the color picker open.

Edit: I seem to get slightly better results with RenderStepped, however it still skips if you go too fast.
Edit2: im bad at titles

You can check if it’s being held when the mouse is moved using the Mouse.Move event. https://developer.roblox.com/en-us/api-reference/event/Mouse/Move

Edit: UserInputService is more powerful and good to know how to use, so you should use Usering’s way instead. The mouse object isn’t technically deprecated, but the wiki says to use UserInputService instead when possible.

1 Like

UserInputService.InputChanged is exactly what you need. Rather than relying on a loop (which would only refresh every 1/30th of a second because of your wait()), this event fires every time the mouse moves.

It’s also not a great idea to fire a remote every time the mouse is moved. If you want to make sure the server saves all changes, store all of these into a table and send the remote every once in a while to make sure the server is up to date. This will decrease the load on the server, especially as more players are drawing.

Example code:

game:GetService("UserInputService").InputChanged:connect(function(input,processed)
	if input.UserInputType == Enum.UserInputType.MouseMovement and not processed then
		print("Mouse has been moved!")
	end
end)
2 Likes

Alright, I’ll look into changing the remote events.

I’ve used your example of InputChanged and it works. (seems to work better than the loop too)
However I am still getting it skipping out on some pixels depending on how fast the mouse is moving.


You can see how it seems to skip pixels less often, but it does still do it.

1 Like

That problem extends past the scope of InputChanged. What you can do to do is “connect the dots” basically. You can generate a line between the dot that has been placed and the previously placed one. This involves some simple math.atan2 and experimentation; if you manage to figure it out, the result will be pretty nice.
@goldenstein64 actually wrote a system that would be more in-line with the theme of a painting program, and would make more sense than my suggestion.

Your best bet is to somehow draw a line of pixels between the last pixel and the current pixel.
Probably the best way to do that is by creating a for loop that starts at the last pixel’s X position and stops at the current X, and just lerp between Y's

-- in a server script

local lastPixel = Vector2.new()

local function DrawLine(currentX, currentY)

	local currentPixel = Vector2.new(currentX, currentY)

	local leftPixel, rightPixel

	if lastPixel.X <= currentPixel.X then
		leftPixel = lastPixel
		rightPixel = currentPixel
	else
		leftPixel = currentPixel
		rightPixel = lastPixel
	end

	local dist = rightPixel.X - leftPixel.X

	for x = 0, dist do
		local a = x/dist

		-- lerp y
		local y = (rightPixel.Y - leftPixel.Y) * a + leftPixel.Y

		-- round y
		y = math.floor(y + 0.5)

		-- server's draw function here
		self:Draw(x + leftPixel.X, y)
	end

	lastPixel = currentPixel
end

This is a nice function and all, but it should in theory only draw 1 pixel per X-position, which is not optimal if the player draws some tall lines. To fix this, we could just change perspective on which axis we are looping on. We can also initialize the lastPixel value to the current value on the first time drawing a pixel so as not to form a line from a corner.

local lastPixel

local function DrawLine(currentX, currentY)

	local currentPixel = Vector2.new(currentX, currentY)

	if not lastPixel then -- lastPixel doesn't exist yet
		lastPixel = currentPixel
		self:Draw(currentPixel.X, currentPixel.Y)
		return
	end
	
	local delta = currentPixel - lastPixel

	local axis, depAxis
	if delta == Vector2.zero then -- no line needs to be drawn
		-- lastPixel already equals currentPixel
		self:Draw(currentPixel.X, currentPixel.Y)
		return
	elseif math.abs(delta.X) >= math.abs(delta.Y) then -- x distance >= y distance
		axis, depAxis = "X", "Y"
	else
		axis, depAxis = "Y", "X"
	end

	local smallPixel, bigPixel
	if math.sign(delta[axis]) == -1 then -- last > current
		smallPixel, bigPixel = currentPixel, lastPixel
	else
		smallPixel, bigPixel = lastPixel, currentPixel
	end

	local deltaAbs = math.abs(delta[axis])
	for i = 1, deltaAbs do
		local a = i/deltaAbs

		-- lerp other axis
		local z = (bigPixel[depAxis] - smallPixel[depAxis]) * a + smallPixel[depAxis]

		-- round other axis
		z = math.floor(z + 0.5)

		if axis == "X" then
			self:Draw(i + smallPixel[axis], z)
		else
			self:Draw(z, i + smallPixel[axis])
		end
	end

	lastPixel = currentPixel
end

The other way of doing this is finding the Y distance between each change in X position by doing delta.X/delta.Y and drawing this many Y pixels per X, but this might cause too few or extra pixels to be drawn based on whether you round, floor, or ceil the resulting number.

If there are any errors in this code, please let me know, as I have not tested it!

Edit: The current code is now glitchless, but it is surprisingly laggy.

Edit 2: Found the reason, it’s evaluating each pixel instead of each frame on my side. Just use a conversion algorithm first!

Edit 3: A bit more cleaned up since last time I updated the code:

local lastPosition

-- uses method Screen:Draw(position: { X: number, Y: number }, color: Color3?)
function Screen:DrawLine(position: Vector2, color: Color3?)
	if lastPosition == nil then
		lastPosition = position
		self:Draw(position, color)
		return
	end
	
	local delta = position - lastPosition

	local axis, depAxis
	if delta == Vector2.zero then -- no line needs to be drawn
		-- lastPixel already equals currentPixel
		self:Draw(position)
		return
	elseif math.abs(delta.X) >= math.abs(delta.Y) then -- x distance >= y distance
		axis, depAxis = "X", "Y"
	else
		axis, depAxis = "Y", "X"
	end

	local smallPixel, bigPixel
	if math.sign(delta[axis]) == -1 then -- last > current
		smallPixel, bigPixel = position, lastPosition
	else
		smallPixel, bigPixel = lastPosition, position
	end

	local deltaAbs = math.abs(delta[axis])
	for i = 0, deltaAbs do
		local a = i/deltaAbs

		-- lerp other axis
		local depValue = bigPixel[depAxis] * a + smallPixel[depAxis] * (1 - a)

		local midPosition = {
			[axis] = i + smallPixel[axis],
			[depAxis] = math.floor(depValue + 0.5)
		}
		
		self:Draw(midPosition, color)
	end

	lastPosition = position
end
4 Likes

Just want to step in a second here and throw down my thread, as I feel it’s relevant here.

This code is a perfect example of how to blatantly abuse conditions for while loops, specifically under yielding at the start of every iteration and thus producing finnicky results. What you should be doing is spawning the loop right when a draw action needs to be performed and terminating it when it no longer has use.

while hold do
    if ColorPicker.Visible then
        Draw:FireServer(mouse.X, mouse.Y) -- Bad for performance
    end
    wait() -- 1/30th second + additional time to search for free TS slot
end

You aren’t using a while loop anymore (which is good) so this is fairly moot, but I wanted to take the time to address that since it hasn’t already been addressed.

3 Likes