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

Zoom: Artifacts in scaling #305 & #18 (from the website project) #400

Closed
itsmedavep opened this issue Jan 27, 2019 · 12 comments · Fixed by #841
Closed

Zoom: Artifacts in scaling #305 & #18 (from the website project) #400

itsmedavep opened this issue Jan 27, 2019 · 12 comments · Fixed by #841
Milestone

Comments

@itsmedavep
Copy link
Contributor

itsmedavep commented Jan 27, 2019

Describe the bug
Per @baconpaul the bitmap scale and the underlying fill scale may be off at zoom levels other than 100%.

This issue references conversations going on in #305 and #18 from the website project

To Reproduce
Steps to reproduce the behavior:

  1. Open Surge
  2. Click menu button
  3. Pick zoom level of 125%
  4. See this:

screen shot 2019-01-27 at 12 34 49 pm

Expected behavior
The grey box should be centered within the black rect.

Screenshots
If applicable, add screenshots to help explain your problem.

Desktop (please complete the following information):

  • OS: macOS 10.14
  • Host: Logic
  • 1.6

Additional context
Im not sure this is related but I see this as well at the same zoom level:
screen shot 2019-01-27 at 12 37 51 pm

The aliasing in the very first screenshot is concerning to me as well. At this zoom level are we still using the 1x assets or are we grabbing 1.5x and downsizing? Im trying to work out why those vectors are aliasing they way they are in that image. Id like to fix if possible on my end. I first need to know what asset were grabbing at the zoom level to start a visual diff on the rendered file vs the one that is being drawn in the GUI.

@itsmedavep itsmedavep changed the title Zoom: Artifacts in scaling #305 & #18 (https://github.com/surge-synthesizer/surge-synthesizer.github.io/issues/18) Zoom: Artifacts in scaling #305 & #18 (from the website project) Jan 27, 2019
@baconpaul
Copy link
Collaborator

At zoom leave of 1.25x on a Retina display we will grab the 3x assets and downsize to 250; and on a non Retina display we will grab the 1.5 and downsize to 1.25.

@itsmedavep
Copy link
Contributor Author

itsmedavep commented Jan 27, 2019

Cheers. Ill crack those open and take a look at what is going on with the aliasing in those rendered assets.

Something about that top screen shot seems off to me. I swear I was enforcing snapping to pixel bounds with the vectors in the design tool.

I know we could be drawing at sub pixels and that could introduce some aliasing but dang does it seems like a lot in that screen grab.

@baconpaul
Copy link
Collaborator

Yeah don’t kill yourself until I get a chance to read the code. I bet we are drawing to a size -1 in float space and should draw to a size -0.5 or some such

Vst3sdk pointer and bitmap arity come first tho

@itsmedavep
Copy link
Contributor Author

I was thinking it could be something like that. macOS requires a similar approach as well.

Drawing controls with Cocoa in macOS requires a half pixel transform and then a backstop of rounding to nearest pixel to keep everything crisp. Or at least it did the last time I was doing much of that type of work.

TL;DR
Ill hold off on the spelunking till its warranted.

@esaruoho
Copy link
Collaborator

fullscreen_30_01_2019__15_44

posting some red arrows here to highlight this weird line that appears below the sliders :)

@baconpaul
Copy link
Collaborator

OK this is a deeply annoying-sh problem

The way this works is it sub-samples bits of image 136 into image 137 trimming and moving it around. Since 137 has white background if that copy in is off by just a little when scaled, it blows up.

So the most correct thing to do - I think - is to bump the black borders in about 1/4 pixel in the 300% image.

Alternately this diff

paul:~/dev/music/surge$ git diff
diff --git a/src/common/gui/CEffectSettings.cpp b/src/common/gui/CEffectSettings.cpp
index bcd319f..740c2f3 100644
--- a/src/common/gui/CEffectSettings.cpp
+++ b/src/common/gui/CEffectSettings.cpp
@@ -67,7 +67,7 @@ void CEffectSettings::draw(CDrawContext* dc)
       for (int i = 0; i < 8; i++)
       {
          CRect r(0, 0, 17, 9);
-         r.offset(size.left, size.top);
+         r.offset(size.left+0.1, size.top);
          r.offset(blocks[i][0], blocks[i][1]);
          int stype = 0;
          if (disabled & (1 << i))

can get us here

screen shot 2019-02-01 at 8 05 23 pm

at a 300% zoom on 300% zoom

Oh and @itsmedavep it seems that arrow going into the plus is still needing vectorizification!

@itsmedavep
Copy link
Contributor Author

Ohhhhhh. Thats what the comment meant!!!! I changed all of the arrows in the graphic out and didnt realize that is mean to be another arrow there.

Just so Im sure were talking about the same thing:

screen shot 2019-02-02 at 11 38 53 am

Its supposed to be an arrow POINTING to the "+". Easy to update.

@baconpaul
Copy link
Collaborator

Yeah or something
Also the line going back to the 0 before seems to be 2 lines.
Basically that whole area is you know. Sort of wonky

@itsmedavep
Copy link
Contributor Author

Yeah or something
Also the line going back to the 0 before seems to be 2 lines.
Basically that whole area is you know. Sort of wonky

Got it! That make sooo much sense. It was a 💡 moment there for me. Cool. Ill fix up now!

@mkruselj
Copy link
Collaborator

mkruselj commented Feb 2, 2019

Possibly related, too - these lines show up at non-100% zoom level over here (W10):

image

@itsmedavep
Copy link
Contributor Author

Thanks, for further context:

screen shot 2019-02-02 at 3 11 22 pm

@baconpaul baconpaul added this to the 1.6.0 milestone Feb 24, 2019
@baconpaul
Copy link
Collaborator

screen shot 2019-02-24 at 9 36 18 am

With the SVG rendering this is mostly fixed, btw. So Lemme couple this to #647

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 a pull request may close this issue.

4 participants