Don't save the current context when drawing shadows. #345

Merged
merged 4 commits into from Mar 15, 2013

Conversation

Projects
None yet
2 participants
Contributor

rdougan commented Mar 14, 2013

Honestly, I don't know enough to know exactly what is going on. But it seems that because drawInContext:context is being called for every shadow, and then again for the actual text.

I removed the Save/Restore context, and then draw the text and shadow all at once. It fixes #344 and after trying the DemoApp, everything looks fine.

Collaborator

odrobnik commented Mar 14, 2013

I don't believe that this is all of it. Is there maybe an unmatched save and restore somewhere?

Contributor

rdougan commented Mar 14, 2013

Nope, they all match up.

Do not drawInContext for each shadow. #344
`drawInContext:context` appears to draw the entire text for each shadow, which means if you have a shadow, the text will be drew twice. This causes strange antialiasing issues which makes the text looks weird.
Contributor

rdougan commented Mar 15, 2013

Figured that the drawInContext is actually drawing the text each time as well as the shadow. Removing that too.

Do you know why this was originally added?

Collaborator

odrobnik commented Mar 15, 2013

That's the only way I knew how to draw multiple shadows.

Von meinem iPhone gesendet

Am Mar 15, 2013 um 5:14 PM schrieb Robert Dougan notifications@github.com:

Figured that the drawInContext is actually drawing the text each time as well as the shadow. Removing that too.

Do you know why this was originally added?


Reply to this email directly or view it on GitHub.

Contributor

rdougan commented Mar 15, 2013

OK, yeah I see. Multiple shadows are now broken.

Collaborator

odrobnik commented Mar 15, 2013

Maybe you can find a method to only draw the shadow?

Von meinem iPhone gesendet

Am Mar 15, 2013 um 5:32 PM schrieb Robert Dougan notifications@github.com:

OK, yeah I see. Multiple shadows are now broken.


Reply to this email directly or view it on GitHub.

Collaborator

odrobnik commented Mar 15, 2013

Maybe investigate: is the text really drawn at the exact same text position?

Von meinem iPhone gesendet

Am Mar 15, 2013 um 5:32 PM schrieb Robert Dougan notifications@github.com:

OK, yeah I see. Multiple shadows are now broken.


Reply to this email directly or view it on GitHub.

Collaborator

odrobnik commented Mar 15, 2013

Maybe try the offset approach mentioned here: http://stackoverflow.com/questions/10608734/draw-shadow-for-transparent-shape

Shift the context by - say - 10000 pixels off and subtract that from the shadow offset.

Von meinem iPhone gesendet

Am Mar 15, 2013 um 5:32 PM schrieb Robert Dougan notifications@github.com:

OK, yeah I see. Multiple shadows are now broken.


Reply to this email directly or view it on GitHub.

Contributor

rdougan commented Mar 15, 2013

Using CGContextBeginTransparencyLayer seems to work:

if (shadows)
{
  CGContextBeginTransparencyLayer(context, nil);

  for (NSDictionary *shadowDict in shadows)
  {
    [self _setShadowInContext:context fromDictionary:shadowDict];
    [oneRun drawInContext:context];
  }

  CGContextEndTransparencyLayer(context);
}

Taken from here: http://stackoverflow.com/a/14114259/273985

Contributor

rdougan commented Mar 15, 2013

However it does mention that it can be expensive.

Collaborator

odrobnik commented Mar 15, 2013

Ah great stuff, test it, restore the context preserving and add it to the pull request please.

Von meinem iPhone gesendet

Am Mar 15, 2013 um 6:25 PM schrieb Robert Dougan notifications@github.com:

Using CGContextBeginTransparencyLayer seems to work:

if (shadows)
{
CGContextBeginTransparencyLayer(context, nil);

for (NSDictionary *shadowDict in shadows)
{
[self _setShadowInContext:context fromDictionary:shadowDict];
[oneRun drawInContext:context];
}

CGContextEndTransparencyLayer(context);
}
Taken from here: http://stackoverflow.com/a/14114259/273985


Reply to this email directly or view it on GitHub.

Contributor

rdougan commented Mar 15, 2013

OK. Turns out that didn't work after all! Tried the offset method and that works.

Contributor

rdougan commented Mar 15, 2013

I hate how dirty it is though.

if (shadows)
{
  CGContextSaveGState(context);
  CGContextSetTextPosition(context, textPosition.x + 10000, textPosition.y);

  for (NSDictionary *shadowDict in shadows)
  {
    [self _setShadowInContext:context fromDictionary:shadowDict];

    [oneRun drawInContext:context];
  }

  CGContextSetTextPosition(context, textPosition.x, textPosition.y);
  CGContextRestoreGState(context);
}

And then in _setShadowInContext:context:

offset.width -= 10000;

CGContextSetShadowWithColor(context, offset, blur, color.CGColor);
Remove CGContextBeginTransparencyLayer for shadows. Instead, give tex…
…t an offset and then remove that same offset from the shadow, making it visible and the text not.
Contributor

rdougan commented Mar 15, 2013

Confirmed working in the DemoApp and in my testcase.

Collaborator

odrobnik commented Mar 15, 2013

I feel a bit uneasy with this myself, because theoretically there could be a very wide view/context and then the text would still be visible. We would have to clip to the area around the glyph run to avoid that.

Yet another suggestion was to use the same color as the shadow for drawing the text

in _setShadow...
CGContextSetShadowWithColor(context, offset, blur, color.CGColor);
CGContextSetFillColorWithColor(context, color.CGColor);

odrobnik added a commit that referenced this pull request Mar 15, 2013

Merge pull request #345 from rdougan/shadow-issue
Don't save the current context when drawing shadows.

@odrobnik odrobnik merged commit f6db538 into Cocoanetics:master Mar 15, 2013

Contributor

rdougan commented Mar 15, 2013

Color unfortunately does not work. Same issue with the antialiasing of the font.

Collaborator

odrobnik commented Mar 15, 2013

I've merged your pull request and I'll add some clipping. Also if there is only a single shadow then I'll probably forego the whole thing.

On Mar 15, 2013, at 7:24 PM, Robert Dougan notifications@github.com wrote:

Color unfortunately does not work. Same issue with the antialiasing of the font.


Reply to this email directly or view it on GitHub.

Contributor

rdougan commented Mar 15, 2013

Alrighty. Thanks!

odrobnik added a commit that referenced this pull request Mar 15, 2013

Optimize Shadow drawing
- Related to #345
- for text with no shadow only the glyph run is drawn
- for text with 1 shadow this is set and the glyph run is drawn
- for text with more than 1 shadow an area is clipped, 100 pixel wider in all directions
  - then the text position is moved outside that area and the shadow gets an additional offset in the other direction
  - this way only the shadow is drawn
  - except for the very last shadow, there the text position is moved back and the glyph run is drawn together with the last shadow

@ghost ghost assigned odrobnik Mar 21, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment