Skip to content

CLX rendering: Add some DVL_ASSUME hints #8584

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.

Sign up for GitHub

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

Merged
glebm merged 1 commit into from
Jun 12, 2026
+14 −2
Merged

CLX rendering: Add some DVL_ASSUME hints #8584

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions CMake/Tests.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,13 @@ add_library(language_for_testing OBJECT test/language_for_testing.cpp)
target_sources(language_for_testing INTERFACE $<TARGET_OBJECTS:language_for_testing>)

target_link_dependencies(codec_test PRIVATE libdevilutionx_codec app_fatal_for_testing)

add_custom_target(clx_render_benchmark_resources
DEPENDS
"${DEVILUTIONX_ASSETS_OUTPUT_DIRECTORY}/data/resistance.clx"
"${DEVILUTIONX_ASSETS_OUTPUT_DIRECTORY}/ui_art/dvl_lrpopup.clx"
)
add_dependencies(clx_render_benchmark clx_render_benchmark_resources)
target_link_dependencies(clx_render_benchmark
PRIVATE
DevilutionX::SDL
Expand Down
1 change: 1 addition & 0 deletions Source/engine/render/clx_render.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,13 +41,13 @@
{
const uint8_t control = *src;
if (!IsClxOpaque(control))
return { src + 1, control };

Check warning on line 44 in Source/engine/render/clx_render.cpp

View workflow job for this annotation

GitHub Actions / tidy-check

Source/engine/render/clx_render.cpp:44:10 [modernize-use-designated-initializers] 

use designated initializer list to initialize 'BlitCommandInfo'
if (IsClxOpaqueFill(control)) {
const uint8_t width = GetClxOpaqueFillWidth(control);
return { src + 2, width };

Check warning on line 47 in Source/engine/render/clx_render.cpp

View workflow job for this annotation

GitHub Actions / tidy-check

Source/engine/render/clx_render.cpp:47:10 [modernize-use-designated-initializers] 

use designated initializer list to initialize 'BlitCommandInfo'
}
const uint8_t width = GetClxOpaquePixelsWidth(control);
return { src + 1 + width, width };

Check warning on line 50 in Source/engine/render/clx_render.cpp

View workflow job for this annotation

GitHub Actions / tidy-check

Source/engine/render/clx_render.cpp:50:9 [modernize-use-designated-initializers] 

use designated initializer list to initialize 'BlitCommandInfo'
}

struct ClipX {
Expand All @@ -56,11 +56,11 @@
int_fast16_t width;
};

DVL_ALWAYS_INLINE DVL_ATTRIBUTE_HOT ClipX CalculateClipX(int_fast16_t x, std::size_t w, const Surface &out)

Check warning on line 59 in Source/engine/render/clx_render.cpp

View workflow job for this annotation

GitHub Actions / tidy-check

Source/engine/render/clx_render.cpp:59:79 [misc-include-cleaner] 

no header providing "std::size_t" is directly included
{
ClipX clip;
clip.left = (x < 0 ? -x : 0);
clip.right = static_cast<int_fast16_t>(static_cast<int_fast16_t>(x + w) > out.w() ? x + w - out.w() : 0);

Check warning on line 63 in Source/engine/render/clx_render.cpp

View workflow job for this annotation

GitHub Actions / tidy-check

Source/engine/render/clx_render.cpp:63:41 [modernize-use-integer-sign-comparison] 

comparison between 'signed' and 'unsigned' integers
clip.width = static_cast<int_fast16_t>(w - clip.left - clip.right);
return clip;
}
Expand Down Expand Up @@ -89,7 +89,7 @@
DVL_ALWAYS_INLINE DVL_ATTRIBUTE_HOT int_fast16_t SkipLinesForRenderBackwardsWithOverrun(
Point &position, RenderSrc &src, int_fast16_t dstHeight)
{
SkipSize skipSize { 0, 0 };

Check warning on line 92 in Source/engine/render/clx_render.cpp

View workflow job for this annotation

GitHub Actions / tidy-check

Source/engine/render/clx_render.cpp:92:20 [modernize-use-designated-initializers] 

use designated initializer list to initialize 'SkipSize'
while (position.y >= dstHeight && src.begin != src.end) {
src.begin = SkipRestOfLineWithOverrun(
src.begin, static_cast<int_fast16_t>(src.width), skipSize);
Expand Down Expand Up @@ -126,6 +126,7 @@
src.begin += v;
}
}
DVL_ASSUME(v >= 1); // All runs (transparent, fill, pixel) are at least 1px wide
dst += v;
remainingWidth -= v;
}
Expand All @@ -137,7 +138,7 @@
}

template <typename BlitFn>
void DoRenderBackwardsClipXY(

Check warning on line 141 in Source/engine/render/clx_render.cpp

View workflow job for this annotation

GitHub Actions / tidy-check

Source/engine/render/clx_render.cpp:141:6 [readability-function-cognitive-complexity] 

function 'DoRenderBackwardsClipXY' has cognitive complexity of 33 (threshold 25)
const Surface &out, Point position, RenderSrc src, ClipX clipX, BlitFn &&blitFn)
{
// Skip the bottom clipped lines.
Expand All @@ -161,7 +162,7 @@
}
while (remainingLeftClip > 0) {
auto [srcEnd, length] = ClxBlitInfo(src.begin);
if (static_cast<int_fast16_t>(length) > remainingLeftClip) {

Check warning on line 165 in Source/engine/render/clx_render.cpp

View workflow job for this annotation

GitHub Actions / tidy-check

Source/engine/render/clx_render.cpp:165:8 [modernize-use-integer-sign-comparison] 

comparison between 'signed' and 'unsigned' integers
const uint8_t control = *src.begin;
const auto overshoot = static_cast<int>(length - remainingLeftClip);
length = std::min<unsigned>(remainingWidth, overshoot);
Expand Down Expand Up @@ -215,7 +216,7 @@

template <typename BlitFn>
void DoRenderBackwards(
const Surface &out, Point position, const uint8_t *src, size_t srcSize,

Check warning on line 219 in Source/engine/render/clx_render.cpp

View workflow job for this annotation

GitHub Actions / tidy-check

Source/engine/render/clx_render.cpp:219:61 [misc-include-cleaner] 

no header providing "size_t" is directly included
unsigned srcWidth, unsigned srcHeight, BlitFn &&blitFn)
{
if (position.y < 0 || position.y + 1 >= static_cast<int>(out.h() + srcHeight))
Expand All @@ -223,7 +224,7 @@
const ClipX clipX = CalculateClipX(position.x, srcWidth, out);
if (clipX.width <= 0)
return;
const RenderSrc srcForBackwards { src, src + srcSize, static_cast<uint_fast16_t>(srcWidth) };

Check warning on line 227 in Source/engine/render/clx_render.cpp

View workflow job for this annotation

GitHub Actions / tidy-check

Source/engine/render/clx_render.cpp:227:34 [modernize-use-designated-initializers] 

use designated initializer list to initialize 'RenderSrc'
if (static_cast<std::size_t>(clipX.width) == srcWidth) {
DoRenderBackwardsClipY(
out, position, srcForBackwards, std::forward<BlitFn>(blitFn));
Expand Down
8 changes: 6 additions & 2 deletions Source/utils/clx_decode.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,9 @@ namespace devilution {

[[nodiscard]] constexpr uint8_t GetClxOpaquePixelsWidth(uint8_t control)
{
return -static_cast<int8_t>(control);
const uint8_t width = -static_cast<int8_t>(control);
DVL_ASSUME(width >= 1 && width <= 65);
return width;
}

[[nodiscard]] constexpr bool IsClxOpaqueFill(uint8_t control)
Expand All @@ -27,7 +29,9 @@ namespace devilution {
[[nodiscard]] constexpr uint8_t GetClxOpaqueFillWidth(uint8_t control)
{
constexpr uint8_t ClxFillEnd = 0xBF;
return ClxFillEnd - control;
const uint8_t width = ClxFillEnd - control;
DVL_ASSUME(width >= 1 && width <= 63);
return width;
}

struct SkipSize {
Expand Down
Loading