superlistapp / super_native_extensions

Native drag & drop, clipboard access and context menu.
MIT License
477 stars 87 forks source link

[ super_drag_and_drop ] macOS ONLY bug, seems like MouseRegion is broken if dragconfig completes after onPointerUp triggered #376

Closed JerContact closed 6 months ago

JerContact commented 6 months ago

This does seem to be a bit of a super_drag_and_drop issue, but unsure if this is in combination with a flutter issue? Anyway, this ONLY happens on macOS, on PC this works fine (though in step 4 the session.dragCompleted still isn't triggered until you move the mouse on PC, but for some reason the icon works as expected), but what the video shows is this:

  1. Mouse Hover Over is in Red, and it stays red if you are "dragging"
  2. I drag outside of the grey region and let go of the mouse, making a completer stop working to trigger the end of the drag config
  3. Notice the mouse quickly changes to point and then back to hand again, and the red icon is somehow still triggered (as well as the mouse icon from the mouse region which is just over a portion of the grey area)
  4. you need to move the mouse to actually get the drag complete to trigger (even though you aren't dragging at all anymore...)

https://github.com/superlistapp/super_native_extensions/assets/38195772/b95061b8-fa6e-418a-abb5-f33731e30b36

EXPECTED: What I would expect, is the once the completer is done, so that the drag config can complete and the mouse is no longer dragging, you would get the session.dragCompleted to trigger, not having to wait for the mouse to move.

Here is the code:

import 'dart:async';
import 'dart:io';
import 'dart:typed_data';
import 'dart:ui' as ui;

import 'package:flutter/material.dart';
import 'package:super_drag_and_drop/super_drag_and_drop.dart';

void main() {
  runApp(const MyApp());
}

class MyApp extends StatelessWidget {
  const MyApp({super.key});

  // This widget is the root of your application.
  @override
  Widget build(BuildContext context) {
    return MaterialApp(
      title: 'Flutter Demo',
      theme: ThemeData(
        colorScheme: ColorScheme.fromSeed(seedColor: Colors.deepPurple),
        useMaterial3: true,
      ),
      home: const MyHomePage(title: 'Flutter Demo Home Page'),
    );
  }
}

class MyHomePage extends StatefulWidget {
  const MyHomePage({super.key, required this.title});

  final String title;

  @override
  State<MyHomePage> createState() => _MyHomePageState();
}

class _MyHomePageState extends State<MyHomePage> {
  int _counter = 0;

  GlobalKey dragKey = GlobalKey();

  bool _complete = false;

  bool _dragging = false;

  /// just making a png as fast as possible
  Future<ui.Image> capturePng(GlobalKey key) async {
    final Completer<ui.Image> imageCompleter = Completer();
    ui.decodeImageFromPixels(
        Uint8List.fromList([0, 0, 0, 0]), 1, 1, ui.PixelFormat.rgba8888,
        (ui.Image img) {
      imageCompleter.complete(img);
    });

    return imageCompleter.future;
  }

  /// A future, or whatever that is doing a bunch of data processing
  /// that can be stopped, but might be a bit after mouse up
  Future<void> _waitForAudioScanComplete() async {
    final completer = Completer<void>();

    for (int i = 0; i < 2; i++) {
      if (_complete) {
        _complete = false;
        break;
      }
      await Future.delayed(Duration(seconds: 1));
    }

    print("completer done with work");

    completer.complete();

    // Wait until the completer is completed
    await completer.future;
  }

  /// drag config
  Future<DragConfiguration?> dragConfig(
      Offset position, DragSession session) async {
    session.dragCompleted.addListener(() async {
      print("done with sessions only after mouse moves");
    });
    List<DragConfigurationItem> configItems = List.empty(growable: true);

    final item = DragItem();

    item.add(Formats.fileUri
        .lazy(() => Uri.file("test", windows: Platform.isWindows)));

    DragConfigurationItem configItem = DragConfigurationItem(
      item: item,
      image: TargetedWidgetSnapshot(
          WidgetSnapshot.image(await capturePng(dragKey)),
          const Rect.fromLTRB(0, 0, 1, 1)),
    );

    await _waitForAudioScanComplete();

    configItems.add(configItem);

    if (configItems.isEmpty) return null;
    return DragConfiguration(items: configItems, allowedOperations: [
      DropOperation.copy,
      DropOperation.link,
      DropOperation.userCancelled
    ]);
  }

  @override
  Widget build(BuildContext context) {
    return Scaffold(
      appBar: AppBar(
        backgroundColor: Theme.of(context).colorScheme.inversePrimary,
        title: Text(widget.title),
      ),
      body: Center(
        child: Column(
          mainAxisAlignment: MainAxisAlignment.center,
          children: <Widget>[
            Container(
              width: 200,
              height: 200,
              child: MouseRegionTest(
                containerWidth: 100,
                dragging: _dragging,
                onPointerDown: (event) {
                  print("onPointerDown");
                  setState(() {
                    _complete = false;
                    _dragging = true;
                  });
                },
                onPointerUp: (event) {
                  print("onPointerUp");
                  setState(() {
                    _complete = true;
                    _dragging = false;
                  });
                },
                dragKey: dragKey,
                dragConfig: dragConfig,
              ),
            ),
          ],
        ),
      ),
    );
  }
}

class MouseRegionTest extends StatefulWidget {
  const MouseRegionTest({
    super.key,
    required this.containerWidth,
    required this.onPointerDown,
    required this.onPointerUp,
    required this.dragKey,
    required this.dragConfig,
    required this.dragging,
  });

  final double containerWidth;

  final void Function(PointerDownEvent event) onPointerDown;

  final void Function(PointerUpEvent event) onPointerUp;

  final GlobalKey dragKey;

  final Future<DragConfiguration?> Function(
      Offset position, DragSession session) dragConfig;

  final bool dragging;

  @override
  State<MouseRegionTest> createState() =>
      _MouseRegionTestState();
}

class _MouseRegionTestState extends State<MouseRegionTest> {
  bool _isMouseHoveringOverDragBox = false;

  @override
  Widget build(BuildContext context) {
    final theme = Theme.of(context);
    final colorScheme = theme.colorScheme;
    final brightness = theme.brightness;
    return Container(
      decoration: BoxDecoration(
          color: brightness == Brightness.light
              ? Colors.black.withOpacity(0.3)
              : Colors.black.withOpacity(0.6),
          border: Border.all(
            width: 1,
            color: brightness == Brightness.light
                ? Colors.white.withOpacity(0.5)
                : Colors.white.withOpacity(0.25),
          ),
          borderRadius: const BorderRadius.all(Radius.circular(10)),
          boxShadow: [
            BoxShadow(
              color: colorScheme.primary
                  .withOpacity(brightness == Brightness.light ? 0.3 : 0.25),
              blurRadius: 7.0,
              spreadRadius: 0.0,
              blurStyle: BlurStyle.outer,
              offset: const Offset(
                0.0,
                0.0,
              ),
            ),
          ]),
      width: widget.containerWidth,
      child: LayoutBuilder(builder: (context, constraints) {
        return Column(
          children: [
            SizedBox(
              height: 100,
              child: Listener(
                onPointerDown: (event) {
                  widget.onPointerDown(event);
                },
                onPointerUp: (event) {
                  widget.onPointerUp(event);
                },
                child: MouseRegion(
                  cursor: SystemMouseCursors.click,
                  onHover: (_) {
                    setState(() {
                      _isMouseHoveringOverDragBox = true;
                    });
                  },
                  onExit: (_) {
                    setState(() {
                      _isMouseHoveringOverDragBox = false;
                    });
                  },
                  child: DragItemWidget(
                    key: widget.dragKey,
                    allowedOperations: () {
                      return [
                        DropOperation.copy,
                      ];
                    },
                    dragItemProvider: (_) {
                      return null;
                    },
                    child: BaseDraggableWidget(
                      dragConfiguration: widget.dragConfig,
                      child: Stack(
                        children: [
                          Container(
                              margin: EdgeInsets.zero,
                              padding: EdgeInsets.zero,
                              decoration: const BoxDecoration(
                                borderRadius: BorderRadius.only(
                                    topLeft: Radius.circular(10),
                                    topRight: Radius.circular(10)),
                                color: Colors.transparent,
                              ),
                              child: Center(
                                  child: Transform.translate(
                                offset: const Offset(0, 0),
                                child: Icon(Icons.drag_handle,
                                    color: _isMouseHoveringOverDragBox ||
                                            widget.dragging
                                        ? Colors.red
                                        : Colors.transparent),
                              ))),
                        ],
                      ),
                    ),
                  ),
                ),
              ),
            ),
            Expanded(
              flex: 1,
              child: MouseRegion(
                onHover: (_) {
                  setState(() {
                    _isMouseHoveringOverDragBox = false;
                  });
                },
                child: Container(
                    margin: EdgeInsets.zero,
                    padding: EdgeInsets.zero,
                    color: Colors.transparent),
              ),
            ),
          ],
        );
      }),
    );
  }
}
knopp commented 6 months ago

The problem with your example is that you're delaying the drag configuration with await _waitForAudioScanComplete();. You need to provide the drag configuration as soon as possible. The function is async to allow for custom widget snapshots for example, but the data must be available immediately. Regardless, this would also be a pretty bad user experience.

JerContact commented 6 months ago

That's not possible to do in our app (immediately have the file), during the drag we are rendering an audio file that then they can drop. We need to have the ability to have it take some amount of time.

The user experience is already duplicated in multiple other apps using this same technique, so in our case, it's expected that it will take a small amount of time to then be able to drop your file

knopp commented 6 months ago

You can't delay the drag. What application have you seen that delays dragging? That's not really a thing. That would result in a really bad user experience.

You can use virtual files, where you can start generating the contents after user performed the drop. This is supported on macOS, Windows and iOS.

JerContact commented 6 months ago

With sound applications both of the top applications, both Soundminuer and Soundly, do this exact implementation when dragging out effected waveforms, i would assume all sound search applications for sound designers will do this, but at the very least the 2 top applications do this and users expect this type of interaction.

The problem with virtual files, from what I understand, is that I need to know the size of the file when dragging, and if I'm rendering the file while dragging, I'm not sure what the size is until I render the file out (and in theory it could be seconds after the user would drop the file in an application). Is that not correct? That's just what I read in one of the threads on github issues with virtual files with super_drag_and_drop.

To give more context, this is a desktop application. As you start to drag we are multi-threading that so the ui isn't being hindered and you see a progress of the render on the app itself, so the user is aware of the rendering happening as they drag it. For most sounds it's really quick, but if you drag 50 mins of sound, it's going to be a few seconds to render that out.

knopp commented 6 months ago

I looked at soundly, I tried it with network conditioner, it seems that they will only let you drag a part of waveform after it has been downloaded locally. At which point writing the temporary file can easily happen synchronously at the beginning of drag.

As for virtual files, I think you should be abe to block at the time where length is needed, that'd be much better experience than blocking the beginning of drag. I think it should also be possible to provde larger than actual length and then just close the stream when finished, I think most applications on windows that support reading from IStream should handle this fine.

JerContact commented 6 months ago

Maybe I'm misunderstanding what you are saying. If I have a local file, and I pitch it down, when i begin a drag, soundly must re-render a new file once I start the drag, as you can see here:

https://github.com/superlistapp/super_native_extensions/assets/38195772/f882032c-a3ef-41fe-973d-ac74cea41a11

It's actually rendering the file after I say starting drag and I'm moving the mouse a bunch. This is an example of a long file that is pitching down the sound.

Are you saying to not use super drag and drop for the initial point down in flutter? To somehow only trigger the drag when it's done rendering? So essentially just switch the widget after it's down loading when mouse is pressed down?

I assume you are saying the same thing for virtual files, or no? Just to put a huge size and let the virtual steam do the rest? I haven't experimented with Virutal files yet, so I'm not sure how they work.

JerContact commented 6 months ago

Here is an example of Soundminer doing this same thing, which is more the implementation we are going with just for context.

https://github.com/superlistapp/super_native_extensions/assets/38195772/58930f81-2f5a-4bb7-a186-56e7f7b0ef36

knopp commented 6 months ago

Right, so after changing the pitch I can see soundly just blocking the UI thread until the file is processed. It's not a great experience tbh, but I see why they'd go for it since it's the easiest. My point was that exporting or processing the file is an operation that has bound to finish in certain time, so you can block the UI even though it's not a good experience, unlike downloading from internet, which may just get stuck and is to guaranteed to finish in any time.

That said, blocking the thread in Flutter is trickier than doing the same think in Qt for various reasons.

If you look at VirtualFileProvider, to start the output you need to call sinkProvider with the expected size. You can delay this call until your file export is finished. Also you can start exporting immediately one the user start the drag, so possibly by the time the file is dropped the export will even be complete. This should not block you UI, it may block the UI of the application you're dropping into until you know the size, but that's still way less disruptive than blocking application UI.

knopp commented 6 months ago

But there's another factor here, if you're expecting to drop the file to particular applications, you might want to ensure that they can actually receive virtual files, which some of them might not, especially if they are cross platform application with incomplete drop support.

JerContact commented 6 months ago

Yeah, that's a good point, I'm not sure I will be able to test all applications they will use, so perhaps virtual isn't the best path. But, if I just use a different widget when the "PointerDown" happens, and give some visual feedback that my app is doing something, and then switch to super_drag_and_drop, perhaps that would work?

Then I wouldn't be messing with taking up the main thread or messing with UI (the c++ portion doing the rendering is multi-threaded anyway, so it doesn't mess with the UI).

knopp commented 6 months ago

You can combine virtual files with a file URL, that's quite commonly done. Applications that can handle virtual files will receive those, other applications can get lower fidelity version from the URL (i.e. iPhoto on macOS does this).

On desktop the drag is initiated from Flutter through gesture detector, but I think the detector currently does not account for cancelling the drag before the data is complete. Might not be too difficult to change that.

knopp commented 6 months ago

I still think it's not a great user experience to delay the drag, but I also think that current behavior is broken and what you're trying to do should work correctly. Can you try this with 0.8.16 once it shows up on pub?

JerContact commented 6 months ago

It works beautifully with 0.8.16! Thanks for looking into this with me!