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

Shadows not working with Mask type. #267

Closed
Abdul-Moiz opened this issue Aug 4, 2016 · 7 comments · Fixed by #295
Closed

Shadows not working with Mask type. #267

Abdul-Moiz opened this issue Aug 4, 2016 · 7 comments · Fixed by #295
Milestone

Comments

@Abdul-Moiz
Copy link

I am trying to apply shadow on Mask type Circle seems like it's not working. Removing mask working properly.

@JakeLin
Copy link
Member

JakeLin commented Aug 5, 2016

@Abdul-Moiz I introduced this bug when I fixed another bug #180 , I am looking at how can we fix it.

@JakeLin
Copy link
Member

JakeLin commented Aug 5, 2016

@Abdul-Moiz I have tried to fix this issue, please have a look at the commit 3c5bc85

But the effect is not right, it is not easy to fix it. If you have time, can you have a look and let me know if you find anything? Thanks

simulator screen shot aug 5 2016 11 45 48 pm

If you want to apply a shadow to a Circle, there is a workaround, you can set cornerRadius = width / 2.

@Abdul-Moiz
Copy link
Author

@JakeLin I tried to look into the issue. Somehow the shadow is clipping within the view bounds.
The approach I can think of to fix it is adding sublayer below main layer. And avoid the masking completely.

For time being going with the workaround 2.

@JakeLin
Copy link
Member

JakeLin commented Aug 9, 2016

@Abdul-Moiz thanks for looking at it. As you said, I think we need to have two layers. And apply the mask to shadowPath of the second layer. Would you like to give it a go with a PR when you have time? Please let me know if you are interested.

@Abdul-Moiz
Copy link
Author

@JakeLin This is the code that made the shadow render correctly on the view with masking. I didn't tested all the cases but working fine for AnimatableView, Button and image.

public func configMaskShadow() {
    commonSetup()
    // if a layer mask is specified, display the shadow to match the mask
    if let mask = layer.mask as? CAShapeLayer {
      // Clear default layer borders
      layer.shadowColor = nil
      layer.shadowRadius = 0
      layer.shadowOpacity = 0

      // Remove any previous layer
      layer.superlayer?.sublayers?.filter { $0.name == "shadowLayer-\(unsafeAddressOf(self))" }
        .forEach { $0.removeFromSuperlayer() }

      // Create new layer with object's memory reference to make this string unique. Otherwise common name will remove all the shadow layers as it's adding in layer's superview.
      let shadowLayer = CAShapeLayer()
      shadowLayer.name = "shadowLayer-\(unsafeAddressOf(self))"
      shadowLayer.frame = frame

      // Configure shadow properties
      if let unwrappedShadowColor = shadowColor {
        shadowLayer.shadowColor = unwrappedShadowColor.CGColor
      }
      if !shadowRadius.isNaN && shadowRadius > 0 {
        shadowLayer.shadowRadius = shadowRadius
      }
      if !shadowOpacity.isNaN && shadowOpacity >= 0 && shadowOpacity <= 1 {
        shadowLayer.shadowOpacity = Float(shadowOpacity)
      }
      if !shadowOffset.x.isNaN {
        shadowLayer.shadowOffset.width = shadowOffset.x
      }
      if !shadowOffset.y.isNaN {
        shadowLayer.shadowOffset.height = shadowOffset.y
      }
      shadowLayer.shadowPath = mask.path

      // Add to layer's superview in order to render shadow otherwise it will clip out due to mask layer.
      layer.superlayer?.insertSublayer(shadowLayer, below: layer)
    }
  }

simulator screen shot aug 11 2016 3 47 04 pm

@JakeLin
Copy link
Member

JakeLin commented Aug 12, 2016

@Abdul-Moiz thanks, looks very good. The solution should be fine. I am busy on preparing a presentation for Dev World will have a deep look later.

@JakeLin
Copy link
Member

JakeLin commented Sep 1, 2016

@Abdul-Moiz Sorry for the late action, I have submitted a PR #295 to address this issue, please have a look when you have time. Thanks for fixing this issue.

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

Successfully merging a pull request may close this issue.

3 participants