Skip to content

Commit c9df651

Browse files
authored
Optimized region drawing and fixed incorrect region clipping (#240)
Made small optimizations to Texture's SetRegion, SetLinearRegion, GetRegion and Clear. They might not be super effective depending on how the compiler already optimized them. The optimizations mainly focus on removing the multiplication when transforming a position to an index by using the row index rather than the y coordinate during iteration. Also fixed regions not drawing properly when clipping against the sides of a window and made GetRegion reject any region that isn't fully inside of the window (previously accepted regions that clipped outside of the right and bottom side, possibly leading to incorrect reads on the user end due to desynchronized region dimensions)
1 parent 2d0c02a commit c9df651

1 file changed

Lines changed: 40 additions & 37 deletions

File tree

runtime/Sources/Renderer/Image.cpp

Lines changed: 40 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -226,49 +226,60 @@ namespace mlx
226226
void Texture::SetRegion(int x, int y, int w, int h, mlx_color* pixels) noexcept
227227
{
228228
MLX_PROFILE_FUNCTION();
229-
if(x < 0 || y < 0 || static_cast<std::uint32_t>(x) >= m_width || static_cast<std::uint32_t>(y) >= m_height)
230-
return;
231-
if(w < 0 || h < 0)
229+
if(w < 0 || h < 0 || x < -w || y < -h
230+
|| x >= static_cast<int>(m_width) || y >= static_cast<int>(m_height))
232231
return;
233232
if(!m_staging_buffer.has_value())
234233
OpenCPUBuffer();
235-
for(std::uint32_t i = 0, moving_x = x, moving_y = y;; i++, moving_x++)
234+
const int
235+
start_x = std::max<int>(x, 0),
236+
start_y = std::max<int>(y, 0),
237+
start_row = start_y * m_width,
238+
start_i = (start_y - y) * w + (start_x - x),
239+
end_x = std::min<int>(x + w, m_width),
240+
end_y = std::min<int>(y + h, m_height),
241+
end_row = end_y * m_width,
242+
incr = (x + w) - end_x + (start_x - x);
243+
for(int i = start_i, dx = start_x, row = start_row;; i++, dx++)
236244
{
237-
if(moving_x >= static_cast<std::uint32_t>(x + w) || moving_x >= m_width)
245+
if(dx >= end_x)
238246
{
239-
moving_x = x;
240-
moving_y++;
241-
if(moving_y >= static_cast<std::uint32_t>(y + h) || moving_y >= m_height)
247+
i += incr;
248+
dx = start_x;
249+
row += m_width;
250+
if(row >= end_row)
242251
break;
243252
}
244253
if constexpr(std::endian::native == std::endian::little)
245-
m_staging_buffer->GetMap<mlx_color*>()[(moving_y * m_width) + moving_x] = ReverseColor(pixels[i]);
254+
m_staging_buffer->GetMap<mlx_color*>()[row + dx] = ReverseColor(pixels[i]);
246255
else
247-
m_staging_buffer->GetMap<mlx_color*>()[(moving_y * m_width) + moving_x] = pixels[i];
256+
m_staging_buffer->GetMap<mlx_color*>()[row + dx] = pixels[i];
248257
}
249258
m_has_been_modified = true;
250259
}
251260

252261
void Texture::SetLinearRegion(int x, int y, std::size_t len, mlx_color* pixels) noexcept
253262
{
254263
MLX_PROFILE_FUNCTION();
255-
if(x < 0 || y < 0 || static_cast<std::uint32_t>(x) >= m_width || static_cast<std::uint32_t>(y) >= m_height)
264+
if(x >= static_cast<int>(m_width) || y >= static_cast<int>(m_height))
256265
return;
257266
if(!m_staging_buffer.has_value())
258267
OpenCPUBuffer();
268+
int
269+
start = y * m_width + x,
270+
dest_start = std::max<int>(start, 0),
271+
src_start = dest_start - start,
272+
dest_end = std::min<int>(start + len, m_width * m_height);
259273
if constexpr(std::endian::native == std::endian::little)
260274
{
261-
for(std::size_t i = 0; i < len && (y * m_width) + x + i < m_width * m_height; i++)
262-
m_staging_buffer->GetMap<mlx_color*>()[(y * m_width) + x + i] = ReverseColor(pixels[i]);
275+
for(int i = dest_start, j = src_start; i < dest_end; i++, j++)
276+
m_staging_buffer->GetMap<mlx_color*>()[i] = ReverseColor(pixels[j]);
263277
}
264278
else
265279
{
266-
std::size_t len_guard;
267-
if((y * m_width + x + len) < m_width * m_height)
268-
len_guard = len;
269-
else
270-
len_guard = len - (m_width * m_height - (y * m_width + x + len));
271-
std::memcpy(&m_staging_buffer->GetMap<mlx_color*>()[(y * m_width) + x], pixels, len_guard);
280+
std::memcpy(
281+
&m_staging_buffer->GetMap<mlx_color*>()[dest_start],
282+
&pixels[src_start], dest_end - dest_start);
272283
}
273284
m_has_been_modified = true;
274285
}
@@ -289,23 +300,23 @@ namespace mlx
289300
void Texture::GetRegion(int x, int y, int w, int h, mlx_color* dst) noexcept
290301
{
291302
MLX_PROFILE_FUNCTION();
292-
if(x < 0 || y < 0 || static_cast<std::uint32_t>(x) >= m_width || static_cast<std::uint32_t>(y) >= m_height)
303+
if(w < 0 || h < 0 || x < 0 || y < 0 || static_cast<std::uint32_t>(x + w) >= m_width || static_cast<std::uint32_t>(y + h) >= m_height)
293304
return;
294305
if(!m_staging_buffer.has_value())
295306
OpenCPUBuffer();
296-
for(std::uint32_t i = 0, moving_x = x, moving_y = y;; i++, moving_x++)
307+
for(std::uint32_t i = 0, dx = x, row = y * m_width;; i++, dx++)
297308
{
298-
if(moving_x >= static_cast<std::uint32_t>(x + w) || moving_x >= m_width)
309+
if(dx >= m_width)
299310
{
300-
moving_x = x;
301-
moving_y++;
302-
if(moving_y >= static_cast<std::uint32_t>(y + h) || moving_y >= m_height)
311+
dx = x;
312+
row += m_width;
313+
if(row >= m_height * m_width)
303314
break;
304315
}
305316
if constexpr(std::endian::native == std::endian::little)
306-
dst[i] = ReverseColor(m_staging_buffer->GetMap<mlx_color*>()[(moving_y * m_width) + moving_x]);
317+
dst[i] = ReverseColor(m_staging_buffer->GetMap<mlx_color*>()[row + dx]);
307318
else
308-
dst[i] = m_staging_buffer->GetMap<mlx_color*>()[(moving_y * m_width) + moving_x];
319+
dst[i] = m_staging_buffer->GetMap<mlx_color*>()[row + dx];
309320
}
310321
}
311322

@@ -320,16 +331,8 @@ namespace mlx
320331
processed_color.g = static_cast<std::uint8_t>(color.g * 255.f);
321332
processed_color.b = static_cast<std::uint8_t>(color.b * 255.f);
322333
processed_color.a = static_cast<std::uint8_t>(color.a * 255.f);
323-
if(processed_color.r == 0 && processed_color.g == 0 && processed_color.b == 0)
324-
std::memset(m_staging_buffer->GetMap(), processed_color.a, m_staging_buffer->GetSize());
325-
else
326-
{
327-
for(std::size_t y = 0; y < m_height; y++)
328-
{
329-
for(std::size_t x = 0; x < m_width; x++)
330-
m_staging_buffer->GetMap<mlx_color*>()[y * m_width + x] = processed_color;
331-
}
332-
}
334+
for(std::size_t i = 0; i < m_width * m_height; i++)
335+
m_staging_buffer->GetMap<mlx_color*>()[i] = processed_color;
333336
}
334337
}
335338

0 commit comments

Comments
 (0)