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

Runtime crash on some devices if a parent has zero scale transform #343

Closed
ghugues opened this issue Oct 6, 2024 · 2 comments
Closed

Runtime crash on some devices if a parent has zero scale transform #343

ghugues opened this issue Oct 6, 2024 · 2 comments

Comments

@ghugues
Copy link

ghugues commented Oct 6, 2024

Description

On at least 1 device that I own (iPhone 12 mini running iOS 17.7), Rive crashes at runtime because of an invalid texture size.

Repro project

Link to sample project : RiveCrashTest.zip

Simplified repro code (note that the project has more details):

class ViewController: UIViewController {

    private let riveModel = RiveViewModel(fileName: "<filename>")

    override func viewDidLoad() {
        super.viewDidLoad()
        view.backgroundColor = .lightGray

        let container = UIView()
        container.translatesAutoresizingMaskIntoConstraints = false
        container.transform = CGAffineTransform(scaleX: 0.0, y: 0.0) // This transform causes the crash.
        view.addSubview(container)

        let riveView = riveModel.createRiveView()
        riveView.translatesAutoresizingMaskIntoConstraints = false
        container.addSubview(riveView)

        NSLayoutConstraint.activate([
            container.centerXAnchor.constraint(equalTo: view.centerXAnchor),
            container.centerYAnchor.constraint(equalTo: view.centerYAnchor),
            container.widthAnchor.constraint(equalToConstant: 100.0),
            container.heightAnchor.constraint(equalToConstant: 100.0),

            riveView.centerXAnchor.constraint(equalTo: view.centerXAnchor),
            riveView.centerYAnchor.constraint(equalTo: view.centerYAnchor),
            riveView.widthAnchor.constraint(equalToConstant: 24.0),
            riveView.heightAnchor.constraint(equalToConstant: 24.0),
        ])
    }
}

Run the application on an iPhone 12 mini with iOS 17.7 and the application will crash after outputting:

Warning: infinite size
Metal view will draw: rect = (0.0, 0.0, 24.0, 24.0); drawableSize = (inf, inf)
-[MTLTextureDescriptorInternal validateWithDevice:], line 1357: error 'Texture Descriptor Validation
MTLTextureDescriptor has width (4294967295) greater than the maximum allowed size of 16384.
MTLTextureDescriptor has height (4294967295) greater than the maximum allowed size of 16384.
'
-[MTLTextureDescriptorInternal validateWithDevice:]:1357: failed assertion `Texture Descriptor Validation
MTLTextureDescriptor has width (4294967295) greater than the maximum allowed size of 16384.
MTLTextureDescriptor has height (4294967295) greater than the maximum allowed size of 16384.
'

The stack trace is the following:

Thread 1 Queue : com.apple.main-thread (serial)
#0	0x00000001d6c982ec in __pthread_kill ()
#1	0x00000001eaa8bc0c in pthread_kill ()
#2	0x0000000195f93ba0 in abort ()
#3	0x0000000195f92eac in __assert_rtn ()
#4	0x000000018dceeaa4 in MTLReportFailure.cold.1 ()
#5	0x000000018db95d40 in MTLReportFailure ()
#6	0x000000018db9566c in _MTLMessageContextEnd ()
#7	0x000000018db94704 in -[MTLTextureDescriptorInternal validateWithDevice:] ()
#8	0x000000022dcef094 in -[AGXTexture initWithDevice:desc:isSuballocDisabled:] ()
#9	0x0000000105f40b60 in rive::gpu::make_pls_memoryless_texture ()
#10	0x0000000105f409e8 in rive::gpu::RenderTargetMetal::RenderTargetMetal ()
#11	0x0000000105f40cc8 in rive::gpu::RenderContextMetalImpl::makeRenderTarget ()
#12	0x0000000105d875c0 in -[RiveRendererContext beginFrame:] ()
#13	0x0000000105d847b8 in -[RiveRendererView drawInRect:withCompletion:] ()
#14	0x0000000105d848ec in -[RiveRendererView drawRect:] ()
#15	0x0000000104dca9fc in CustomRiveView.draw(_:) at .../AppDelegate.swift:86
#16	0x0000000104dcab18 in @objc CustomRiveView.draw(_:) ()
#17	0x0000000203b16494 in -[MTKView draw] ()
#18	0x0000000203b163f4 in -[MTKViewDisplayLinkTarget draw] ()
#19	0x000000018f6f0dc4 in CA::Display::DisplayLinkItem::dispatch_ ()
#20	0x000000018f6ef0e8 in CA::Display::DisplayLink::dispatch_items ()
#21	0x000000018f6eec88 in CA::Display::DisplayLink::dispatch_deferred_display_links ()
#22	0x00000001902e7660 in _UIUpdateSequenceRun ()
#23	0x00000001902e72a4 in schedulerStepScheduledMainSection ()
#24	0x00000001902e8148 in runloopSourceCallback ()
#25	0x000000018e003834 in __CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION__ ()
#26	0x000000018e0037c8 in __CFRunLoopDoSource0 ()
#27	0x000000018e0012f8 in __CFRunLoopDoSources0 ()
#28	0x000000018e000484 in __CFRunLoopRun ()
#29	0x000000018dfffcd8 in CFRunLoopRunSpecific ()
#30	0x00000001d2a551a8 in GSEventRunModal ()
#31	0x000000019063aae8 in -[UIApplication _run] ()
#32	0x00000001906eed98 in UIApplicationMain ()
#33	0x0000000190868504 in ___lldb_unnamed_symbol261261 ()
#34	0x0000000104dc8920 in static UIApplicationDelegate.main() ()
#35	0x0000000104dc8898 in static AppDelegate.$main() ()
#36	0x0000000104dcba7c in main at .../AppDelegate.swift:5
#37	0x00000001b17df154 in start ()

Notice the scale transform with a scale of zero on the parent view of the rive view. This is the reason for the invalid drawableSize. Without this transform, everything works as expected. The same problem occurs if the transform is on any parent view up the hierarchy, it doesn't have to be the direct superview.

Expected behavior

The application should not crash. Because it is unclear why the drawableSize has an infinite size on some devices (and may be a bug in the Metal framework), the rive metal view should perform checks on the drawableSize before drawing. In the same way it already checks that the size is not zero, it should also check for infinite, nan, etc. values that could cause a crash down the line.

Edit: After further testing, it appears that using a zero scale is just an extreme case of a broader issue. Any small scale transform on a parent view will produce a drawable size larger than the maximum texture size supported by the GPU and trigger a crash (even a 0.1 scale can trigger this crash). So although proper checks on the drawable size will avoid a crash, this could still leave the door open to performance problems by servicing a drawable size way bigger than actually necessary.

Device & Versions

The crash occurs at least on an iPhone 12 mini running iOS 17.7.
The crash does not occur on an iPhone 14 Pro running iOS 17.2.1.
Without additional testing on more devices, it is unclear what combination of device and iOS version causes the crash.

@dskuza
Copy link
Collaborator

dskuza commented Oct 10, 2024

@ghugues Thanks for sending this over, and providing a sample app. I also appreciate the open pull request, it's helpful insight. I've brought this up internally to decide where the right fix it. It could be fixed at the iOS runtime level, as you've showcased in your pull request, but there might be a lower-level fix we could also provide. Just double checking with the team.

rive-engineering pushed a commit that referenced this issue Oct 18, 2024
This pull request fixes some crashes (see [rive-ios #343](#343)) where the (drawable) size of the underlying view was not within the bounds within which Metal is capable of drawing.

Possible conditions:
- Width or height of the view is 0
- Transform of the view is x / y scale 0

The maximum allowed drawable sizes are based on [this Apple document](https://developer.apple.com/metal/Metal-Feature-Set-Tables.pdf). See: page 7.

Essentially, before every draw, we now check whether we _can_ draw via Metal based on the size of the view, as well as the provided drawable size of the underlying Metal view. If not, we do an early return and no drawing happens.

Unit tests have also been added for a few main scenarios under which this could occur.

Diffs=
fab89ebbba Validate rect and drawable size before drawing on iOS (#8334)

Co-authored-by: David Skuza <[email protected]>
@dskuza
Copy link
Collaborator

dskuza commented Oct 22, 2024

@ghugues This should now be fixed in v6.2.1, released earlier today.

@dskuza dskuza closed this as completed Dec 23, 2024
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

No branches or pull requests

2 participants