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

Replace Dropzone VTKActor by IMGUI #1990

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

Ni-g-3l
Copy link
Contributor

@Ni-g-3l Ni-g-3l commented Feb 15, 2025

Hello !

I try to replace DropZone Actor by a IMGUI window. I'm not ImGUI expert if you see a better way to do it do not hesitate to comment.

image

Linked issue : #1975

@mwestphal
Copy link
Contributor

Do you think you could put an outline around the text, like its done for the filename ?

@mwestphal
Copy link
Contributor

Other than that, its looking good! Dont hesitate to share such screenshot on discord :)

@mwestphal
Copy link
Contributor

However, how can I update tests in order to match the new dropzone ?

You just need to update the baselines

@Ni-g-3l
Copy link
Contributor Author

Ni-g-3l commented Feb 15, 2025

Sorry I don't understand ? Where I can find the example to put outline on text ? because my update on DropText look very similar at what it is done for FileName

@mwestphal
Copy link
Contributor

I think its handled by SetupNextWindow

@mwestphal
Copy link
Contributor

mwestphal commented Feb 15, 2025

I mean this:
a

@snoyer
Copy link
Contributor

snoyer commented Feb 16, 2025

@Ni-g-3l I would suggest trying absurdly high values for tickThickness and tickLength in order to highlight accuracy/precision issues in the drawing logic.

For example, using:

    constexpr float tickThickness = 25.0f;
    constexpr float tickLength = 40.0f;

this PR's ImGui code shows:
image
while the previous VTK code shows:
Screenshot From 2025-02-16 09-25-07

The key point here is that the previous code was drawing rectangles and the new code is drawing thick lines so there's going to be some ±thickness/2 offsets to account for somewhere somehow

(also check to make sure it all still works when resizing the window)

possible fix/improvement
diff --git a/vtkext/private/module/vtkF3DImguiActor.cxx b/vtkext/private/module/vtkF3DImguiActor.cxx
index b54e941c..c386d537 100644
--- a/vtkext/private/module/vtkF3DImguiActor.cxx
+++ b/vtkext/private/module/vtkF3DImguiActor.cxx
@@ -299,8 +299,8 @@ void vtkF3DImguiActor::RenderDropZone()
     constexpr float tickLength = 10.0f;
 
     int dropzonePad = static_cast<int>(std::min(viewport->WorkSize.x, viewport->WorkSize.y) * 0.1);
-    int dropZoneW = viewport->WorkSize.x - dropzonePad;
-    int dropZoneH = viewport->WorkSize.y - dropzonePad;
+    int dropZoneW = viewport->WorkSize.x - dropzonePad * 2;
+    int dropZoneH = viewport->WorkSize.y - dropzonePad * 2;
 
     ::SetupNextWindow(ImVec2(0, 0), viewport->WorkSize);
     ImGui::SetNextWindowBgAlpha(0.f);
@@ -312,7 +312,7 @@ void vtkF3DImguiActor::RenderDropZone()
     ImDrawList* draw_list = ImGui::GetWindowDrawList();
 
     ImVec2 p0(dropzonePad, dropzonePad);
-    ImVec2 p1(dropZoneW, dropZoneH);
+    ImVec2 p1(dropzonePad + dropZoneW, dropzonePad + dropZoneH);
     int tickNumberW = static_cast<int>(std::ceil(dropZoneW / (tickLength * 2.0f)));
     int tickNumberH = static_cast<int>(std::ceil(dropZoneH / (tickLength * 2.0f)));
     double tickSpaceW =
@@ -323,22 +323,24 @@ void vtkF3DImguiActor::RenderDropZone()
     // Draw top and bottom line
     for (float x = p0.x; x < p1.x; x += tickLength + tickSpaceW)
     {
-      draw_list->AddLine(ImVec2(x, p0.y), ImVec2(x + tickLength, p0.y), color, tickThickness);
-      draw_list->AddLine(ImVec2(x, p1.y), ImVec2(x + tickLength, p1.y), color, tickThickness);
+      const float y0 = p0.y + tickThickness / 2.f;
+      const float y1 = p1.y - tickThickness / 2.f;
+      draw_list->AddLine(ImVec2(x, y0), ImVec2(x + tickLength, y0), color, tickThickness);
+      draw_list->AddLine(ImVec2(x, y1), ImVec2(x + tickLength, y1), color, tickThickness);
     }
 
     // Draw left and right line
     for (float y = p0.y; y < p1.y; y += tickLength + tickSpaceH)
     {
-      draw_list->AddLine(ImVec2(p0.x, y), ImVec2(p0.x, y + tickLength), color, tickThickness);
-      draw_list->AddLine(ImVec2(p1.x, y), ImVec2(p1.x, y + tickLength), color, tickThickness);
+      const float x0 = p0.x + tickThickness / 2.f;
+      const float x1 = p1.x - tickThickness / 2.f;
+      draw_list->AddLine(ImVec2(x0, y), ImVec2(x0, y + tickLength), color, tickThickness);
+      draw_list->AddLine(ImVec2(x1, y), ImVec2(x1, y + tickLength), color, tickThickness);
     }
 
     ImGui::End();
 
     ImVec2 dropTextSize = ImGui::CalcTextSize(this->DropText.c_str());
-    dropTextSize.x += 2.f * ImGui::GetStyle().WindowPadding.x;
-    dropTextSize.y += 2.f * ImGui::GetStyle().WindowPadding.y;
 
     ImGui::Begin("DropZoneText", nullptr, flags);
     ImGui::SetCursorPos(ImVec2(viewport->GetWorkCenter().x - 0.5f * dropTextSize.x,

@Ni-g-3l
Copy link
Contributor Author

Ni-g-3l commented Feb 16, 2025

I try to fix what you find, with my latest commit

image

image

image

image

I will fix the CICD tomorow

Copy link
Contributor

@snoyer snoyer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Ni-g-3l I believe you forgot to double the padding at the beginning and ended up trying to compensate for that all along later

Comment on lines +323 to +348
double tickBBSizeW = tickLength + tickSpaceW;
double tickBBSizeH = tickLength + tickSpaceH;

// Draw top and bottom line
for (float x = p0.x - tickHalfThickness; x < p1.x - tickBBSizeW; x += tickBBSizeW)
{
draw_list->AddLine(ImVec2(x, p0.y), ImVec2(x + tickLength, p0.y), color, tickThickness);
draw_list->AddLine(ImVec2(x, p1.y), ImVec2(x + tickLength, p1.y), color, tickThickness);
}

draw_list->AddLine(ImVec2(p1.x - tickLength, p0.y), ImVec2(p1.x + tickHalfThickness, p0.y),
color, tickThickness);
draw_list->AddLine(ImVec2(p1.x - tickLength, p1.y), ImVec2(p1.x + tickHalfThickness, p1.y),
color, tickThickness);

// Draw left and right line
for (float y = p0.y - tickHalfThickness; y < p1.y - tickBBSizeH; y += tickBBSizeH)
{
draw_list->AddLine(ImVec2(p0.x, y), ImVec2(p0.x, y + tickLength), color, tickThickness);
draw_list->AddLine(ImVec2(p1.x, y), ImVec2(p1.x, y + tickLength), color, tickThickness);
}

draw_list->AddLine(ImVec2(p0.x, p1.y - tickLength), ImVec2(p0.x, p1.y + tickHalfThickness),
color, tickThickness);
draw_list->AddLine(ImVec2(p1.x, p1.y - tickLength), ImVec2(p1.x, p1.y + tickHalfThickness),
color, tickThickness);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the half-thickness offset you want is only to draw the lines inside the target rectangle rather that on it:
Screenshot From 2025-02-17 06-44-08
vs
Screenshot From 2025-02-17 06-44-31

So you just have to to shift the top/bottom lines down/up and the right/left lines left/right, no need to modify/extend the loops:

Suggested change
double tickBBSizeW = tickLength + tickSpaceW;
double tickBBSizeH = tickLength + tickSpaceH;
// Draw top and bottom line
for (float x = p0.x - tickHalfThickness; x < p1.x - tickBBSizeW; x += tickBBSizeW)
{
draw_list->AddLine(ImVec2(x, p0.y), ImVec2(x + tickLength, p0.y), color, tickThickness);
draw_list->AddLine(ImVec2(x, p1.y), ImVec2(x + tickLength, p1.y), color, tickThickness);
}
draw_list->AddLine(ImVec2(p1.x - tickLength, p0.y), ImVec2(p1.x + tickHalfThickness, p0.y),
color, tickThickness);
draw_list->AddLine(ImVec2(p1.x - tickLength, p1.y), ImVec2(p1.x + tickHalfThickness, p1.y),
color, tickThickness);
// Draw left and right line
for (float y = p0.y - tickHalfThickness; y < p1.y - tickBBSizeH; y += tickBBSizeH)
{
draw_list->AddLine(ImVec2(p0.x, y), ImVec2(p0.x, y + tickLength), color, tickThickness);
draw_list->AddLine(ImVec2(p1.x, y), ImVec2(p1.x, y + tickLength), color, tickThickness);
}
draw_list->AddLine(ImVec2(p0.x, p1.y - tickLength), ImVec2(p0.x, p1.y + tickHalfThickness),
color, tickThickness);
draw_list->AddLine(ImVec2(p1.x, p1.y - tickLength), ImVec2(p1.x, p1.y + tickHalfThickness),
color, tickThickness);
// Draw top and bottom line
for (float x = p0.x; x < p1.x; x += tickLength + tickSpaceW)
{
const float y0 = p0.y + tickThickness / 2.f;
const float y1 = p1.y - tickThickness / 2.f;
draw_list->AddLine(ImVec2(x, y0), ImVec2(x + tickLength, y0), color, tickThickness);
draw_list->AddLine(ImVec2(x, y1), ImVec2(x + tickLength, y1), color, tickThickness);
}
// Draw left and right line
for (float y = p0.y; y < p1.y; y += tickLength + tickSpaceH)
{
const float x0 = p0.x + tickThickness / 2.f;
const float x1 = p1.x - tickThickness / 2.f;
draw_list->AddLine(ImVec2(x0, y), ImVec2(x0, y + tickLength), color, tickThickness);
draw_list->AddLine(ImVec2(x1, y), ImVec2(x1, y + tickLength), color, tickThickness);
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants