zan1456/flutter
1
0
Fork 0
forked from firka/flutter
Code Pull Requests Activity

[macOS] Change view ID to signed (flutter/engine#39958)

Browse Source 
This PR makes view ID signed from unsigned int64.

Initially, I made view IDs unsigned because they were opaque anyway. As
I'm working deeper into multiview, I found some issues that made me
think signed is better:

* Unsigned integers are worse
  * Sometimes you want negative values to represent special values.
* Unsigned integers are dangerous (if compared with signed ones by
mistake.)
* Unsigned integers are not needed
  * We're very unlikely to reach that big anyway.
  * Almost all other languages support only signed integers.
  * Also JavaScript only supports up to 51 bits of integer.

Therefore I think it's better to change them to signed int64, especially
before these APIs are widely used by developers.

## Pre-launch Checklist

- [ ] I read the [Contributor Guide] and followed the process outlined
there for submitting PRs.
- [ ] I read the [Tree Hygiene] wiki page, which explains my
responsibilities.
- [ ] I read and followed the [Flutter Style Guide] and the [C++,
Objective-C, Java style guides].
- [ ] I listed at least one issue that this PR fixes in the description
above.
- [ ] I added new tests to check the change I am making or feature I am
adding, or Hixie said the PR is test-exempt. See [testing the engine]
for instructions on writing and running engine tests.
- [ ] I updated/added relevant documentation (doc comments with `///`).
- [ ] I signed the [CLA].
- [ ] All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel
on [Discord].

<!-- Links -->
[Contributor Guide]:
https://github.com/flutter/flutter/wiki/Tree-hygiene#overview
[Tree Hygiene]: https://github.com/flutter/flutter/wiki/Tree-hygiene
[Flutter Style Guide]:
https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo
[C++, Objective-C, Java style guides]:
https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
[testing the engine]:
https://github.com/flutter/flutter/wiki/Testing-the-engine
[CLA]: https://cla.developers.google.com/
[flutter/tests]: https://github.com/flutter/tests
[breaking change policy]:
https://github.com/flutter/flutter/wiki/Tree-hygiene#handling-breaking-changes
[Discord]: https://github.com/flutter/flutter/wiki/Chat
This commit is contained in:
Tong Mu
2023-03-31 12:11:49 -07:00
committed by GitHub
parent bd145d476a
commit d3bbe5cb8a
22 changed files with 60 additions and 45 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
engine/src/flutter/lib/ui/platform_dispatcher.dart
View File

@@ -390,9 +390,8 @@ class PlatformDispatcher {
// If this value changes, update the encoding code in the following files:
//
// * pointer_data.cc
// * pointer.dart
// * AndroidTouchProcessor.java
static const int _kPointerDataFieldCount = 36;
static const int _kPointerDataFieldCount = 37;
static PointerDataPacket _unpackPointerDataPacket(ByteData packet) {
const int kStride = Int64List.bytesPerElement;
@@ -438,6 +437,7 @@ class PlatformDispatcher {
panDeltaY: packet.getFloat64(kStride * offset++, _kFakeHostEndian),
scale: packet.getFloat64(kStride * offset++, _kFakeHostEndian),
rotation: packet.getFloat64(kStride * offset++, _kFakeHostEndian),
viewId: packet.getInt64(kStride * offset++, _kFakeHostEndian),
preferredStylusAuxiliaryAction: PointerPreferredStylusAuxiliaryAction.values[packet.getInt64(kStride * offset++, _kFakeHostEndian)],
));
assert(offset == (i + 1) * _kPointerDataFieldCount);

5
engine/src/flutter/lib/ui/pointer.dart
View File

@@ -197,6 +197,7 @@ class PointerData {
this.panDeltaY = 0.0,
this.scale = 0.0,
this.rotation = 0.0,
this.viewId = 0,
this.preferredStylusAuxiliaryAction = PointerPreferredStylusAuxiliaryAction.ignore,
});
@@ -396,6 +397,9 @@ class PointerData {
/// The current angle of the pan/zoom in radians, with 0.0 as the initial angle.
final double rotation;
/// The ID of the view that this event took place.
final int viewId;
/// For events with signal kind of stylusAuxiliaryAction
///
/// The current preferred action for stylusAuxiliaryAction, with ignore as the default.
@@ -441,6 +445,7 @@ class PointerData {
'panDeltaY: $panDeltaY, '
'scale: $scale, '
'rotation: $rotation, '
'viewId: $viewId, '
'preferredStylusAuxiliaryAction: $preferredStylusAuxiliaryAction'
')';
}

7
engine/src/flutter/lib/ui/window/pointer_data.h
View File

@@ -9,9 +9,9 @@
namespace flutter {
// If this value changes, update the pointer data unpacking code in
// platform_dispatcher.dart.
static constexpr int kPointerDataFieldCount = 36;
// If this value changes, other places need changing too. See
// _kPointerDataFieldCount in platform_dispatcher.dart.
static constexpr int kPointerDataFieldCount = 37;
static constexpr int kBytesPerField = sizeof(int64_t);
// Must match the button constants in events.dart.
enum PointerButtonMouse : int64_t {
@@ -110,6 +110,7 @@ struct alignas(8) PointerData {
double pan_delta_y;
double scale;
double rotation;
int64_t view_id;
PreferredStylusAuxiliaryAction preferred_auxiliary_stylus_action;
void Clear();

3
engine/src/flutter/lib/web_ui/lib/pointer.dart
View File

@@ -89,6 +89,7 @@ class PointerData {
this.panDeltaY = 0.0,
this.scale = 0.0,
this.rotation = 0.0,
this.viewId = 0,
this.preferredStylusAuxiliaryAction = PointerPreferredStylusAuxiliaryAction.ignore,
});
final int embedderId;
@@ -126,6 +127,7 @@ class PointerData {
final double panDeltaY;
final double scale;
final double rotation;
final int viewId;
final PointerPreferredStylusAuxiliaryAction preferredStylusAuxiliaryAction;
@override
@@ -166,6 +168,7 @@ class PointerData {
'panDeltaY: $panDeltaY, '
'scale: $scale, '
'rotation: $rotation, '
'viewId: $viewId, '
'preferredStylusAuxiliaryAction: $preferredStylusAuxiliaryAction'
')';
}

9
engine/src/flutter/shell/platform/android/io/flutter/embedding/android/AndroidTouchProcessor.java
View File

@@ -94,8 +94,9 @@ public class AndroidTouchProcessor {
int UNKNOWN = 4;
}
// Must match the unpacking code in hooks.dart.
private static final int POINTER_DATA_FIELD_COUNT = 36;
// If this value changes, other places need changing too. See
// _kPointerDataFieldCount in platform_dispatcher.dart.
private static final int POINTER_DATA_FIELD_COUNT = 37;
@VisibleForTesting static final int BYTES_PER_FIELD = 8;
// This value must match the value in framework's platform_view.dart.
@@ -238,6 +239,8 @@ public class AndroidTouchProcessor {
int pointerData,
Matrix transformMatrix,
ByteBuffer packet) {
// TODO(dkwingsmt): Get the source view ID from event data.
int viewId = 0;
if (pointerChange == -1) {
return;
}
@@ -373,6 +376,8 @@ public class AndroidTouchProcessor {
packet.putDouble(1.0); // scale
packet.putDouble(0.0); // rotation
packet.putLong(viewId); // viewId
packet.putLong(PointerPreferredStylusAuxiliaryAction.IGNORE); // preferred stylus action
if (isTrackpadPan && getPointerChangeForPanZoom(pointerChange) == PointerChange.PAN_ZOOM_END) {

2
engine/src/flutter/shell/platform/darwin/macos/framework/Headers/FlutterEngine.h
View File

@@ -26,7 +26,7 @@
* backward compatibility, single-view APIs will always operate on the view with
* this ID. Also, the first view assigned to the engine will also have this ID.
*/
extern const uint64_t kFlutterDefaultViewId;
extern const int64_t kFlutterDefaultViewId;
@class FlutterViewController;

7
engine/src/flutter/shell/platform/darwin/macos/framework/Headers/FlutterPluginRegistrarMacOS.h
View File

@@ -4,6 +4,8 @@
#import <Cocoa/Cocoa.h>
#include <stdint.h>
#import "FlutterBinaryMessenger.h"
#import "FlutterChannels.h"
#import "FlutterMacros.h"
@@ -11,6 +13,9 @@
#import "FlutterPluginMacOS.h"
#import "FlutterTexture.h"
typedef int64_t FlutterViewId;
constexpr int64_t kFlutterDefaultViewId = 0;
// TODO: Merge this file and FlutterPluginMacOS.h with the iOS FlutterPlugin.h, sharing all but
// the platform-specific methods.
@@ -47,7 +52,7 @@ FLUTTER_DARWIN_EXPORT
/**
* The `NSView` associated with the given view ID, if any.
*/
- (nullable NSView*)viewForId:(uint64_t)viewId;
- (nullable NSView*)viewForId:(int64_t)viewId;
/**
* Registers |delegate| to receive handleMethodCall:result: callbacks for the given |channel|.

2
engine/src/flutter/shell/platform/darwin/macos/framework/Headers/FlutterViewController.h
View File

@@ -61,7 +61,7 @@ FLUTTER_DARWIN_EXPORT
* If the view controller is unattached (see FlutterViewController#attached),
* reading this property throws an assertion.
*/
@property(nonatomic, readonly) uint64_t viewId;
@property(nonatomic, readonly) int64_t viewId;
/**
* The style of mouse tracking to use for the view. Defaults to

2
engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterCompositor.h
View File

@@ -49,7 +49,7 @@ class FlutterCompositor {
// Presents the FlutterLayers by updating the FlutterView specified by
// `view_id` using the layer content. Sets frame_started_ to false.
bool Present(uint64_t view_id, const FlutterLayer** layers, size_t layers_count);
bool Present(int64_t view_id, const FlutterLayer** layers, size_t layers_count);
private:
void PresentPlatformViews(FlutterView* default_base_view,

4
engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterCompositor.mm
View File

@@ -37,9 +37,7 @@ bool FlutterCompositor::CreateBackingStore(const FlutterBackingStoreConfig* conf
return true;
}
bool FlutterCompositor::Present(uint64_t view_id,
const FlutterLayer** layers,
size_t layers_count) {
bool FlutterCompositor::Present(int64_t view_id, const FlutterLayer** layers, size_t layers_count) {
FlutterView* view = [view_provider_ viewForId:view_id];
if (!view) {
return false;

4
engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterCompositorTest.mm
View File

@@ -11,7 +11,7 @@
#import "flutter/shell/platform/darwin/macos/framework/Source/FlutterViewProvider.h"
#import "flutter/testing/testing.h"
extern const uint64_t kFlutterDefaultViewId;
extern const int64_t kFlutterDefaultViewId;
@interface FlutterViewMockProvider : NSObject <FlutterViewProvider> {
FlutterView* _defaultView;
@@ -32,7 +32,7 @@ extern const uint64_t kFlutterDefaultViewId;
return self;
}
- (nullable FlutterView*)viewForId:(uint64_t)viewId {
- (nullable FlutterView*)viewForId:(int64_t)viewId {
if (viewId == kFlutterDefaultViewId) {
return _defaultView;
}

16
engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterEngine.mm
View File

@@ -25,8 +25,6 @@
#import "flutter/shell/platform/darwin/macos/framework/Source/FlutterViewController_Internal.h"
#import "flutter/shell/platform/darwin/macos/framework/Source/FlutterViewEngineProvider.h"
const uint64_t kFlutterDefaultViewId = 0;
NSString* const kFlutterPlatformChannel = @"flutter/platform";
/**
@@ -90,7 +88,7 @@ constexpr char kTextPlainFormat[] = "text/plain";
*/
@property(nonatomic, strong) NSMutableArray<NSNumber*>* isResponseValid;
- (nullable FlutterViewController*)viewControllerForId:(uint64_t)viewId;
- (nullable FlutterViewController*)viewControllerForId:(int64_t)viewId;
/**
* An internal method that adds the view controller with the given ID.
@@ -98,7 +96,7 @@ constexpr char kTextPlainFormat[] = "text/plain";
* This method assigns the controller with the ID, puts the controller into the
* map, and does assertions related to the default view ID.
*/
- (void)registerViewController:(FlutterViewController*)controller forId:(uint64_t)viewId;
- (void)registerViewController:(FlutterViewController*)controller forId:(int64_t)viewId;
/**
* An internal method that removes the view controller with the given ID.
@@ -107,7 +105,7 @@ constexpr char kTextPlainFormat[] = "text/plain";
* map. This is an no-op if the view ID is not associated with any view
* controllers.
*/
- (void)deregisterViewControllerForId:(uint64_t)viewId;
- (void)deregisterViewControllerForId:(int64_t)viewId;
/**
* Shuts down the engine if view requirement is not met, and headless execution
@@ -294,7 +292,7 @@ constexpr char kTextPlainFormat[] = "text/plain";
return [self viewForId:kFlutterDefaultViewId];
}
- (NSView*)viewForId:(uint64_t)viewId {
- (NSView*)viewForId:(int64_t)viewId {
FlutterViewController* controller = [_flutterEngine viewControllerForId:viewId];
if (controller == nil) {
return nil;
@@ -573,7 +571,7 @@ static void OnPlatformMessage(const FlutterPlatformMessage* message, FlutterEngi
}
}
- (void)registerViewController:(FlutterViewController*)controller forId:(uint64_t)viewId {
- (void)registerViewController:(FlutterViewController*)controller forId:(int64_t)viewId {
NSAssert(controller != nil, @"The controller must not be nil.");
NSAssert(![controller attached],
@"The incoming view controller is already attached to an engine.");
@@ -583,7 +581,7 @@ static void OnPlatformMessage(const FlutterPlatformMessage* message, FlutterEngi
[_viewControllers setObject:controller forKey:@(viewId)];
}
- (void)deregisterViewControllerForId:(uint64_t)viewId {
- (void)deregisterViewControllerForId:(int64_t)viewId {
FlutterViewController* oldController = [self viewControllerForId:viewId];
if (oldController != nil) {
[oldController detachFromEngine];
@@ -597,7 +595,7 @@ static void OnPlatformMessage(const FlutterPlatformMessage* message, FlutterEngi
}
}
- (FlutterViewController*)viewControllerForId:(uint64_t)viewId {
- (FlutterViewController*)viewControllerForId:(int64_t)viewId {
FlutterViewController* controller = [_viewControllers objectForKey:@(viewId)];
NSAssert(controller == nil || controller.viewId == viewId,
@"The stored controller has unexpected view ID.");

10
engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterEngineTest.mm
View File

@@ -647,7 +647,7 @@ TEST_F(FlutterEngineTest, ManageControllersIfInitiatedByController) {
@autoreleasepool {
// Create FVC1.
viewController1 = [[FlutterViewController alloc] initWithProject:project];
EXPECT_EQ(viewController1.viewId, 0ull);
EXPECT_EQ(viewController1.viewId, 0ll);
engine = viewController1.engine;
engine.viewController = nil;
@@ -664,7 +664,7 @@ TEST_F(FlutterEngineTest, ManageControllersIfInitiatedByController) {
engine.viewController = viewController1;
EXPECT_EQ(engine.viewController, viewController1);
EXPECT_EQ(viewController1.viewId, 0ull);
EXPECT_EQ(viewController1.viewId, 0ll);
}
TEST_F(FlutterEngineTest, ManageControllersIfInitiatedByEngine) {
@@ -678,7 +678,7 @@ TEST_F(FlutterEngineTest, ManageControllersIfInitiatedByEngine) {
@autoreleasepool {
viewController1 = [[FlutterViewController alloc] initWithEngine:engine nibName:nil bundle:nil];
EXPECT_EQ(viewController1.viewId, 0ull);
EXPECT_EQ(viewController1.viewId, 0ll);
EXPECT_EQ(engine.viewController, viewController1);
engine.viewController = nil;
@@ -686,7 +686,7 @@ TEST_F(FlutterEngineTest, ManageControllersIfInitiatedByEngine) {
FlutterViewController* viewController2 = [[FlutterViewController alloc] initWithEngine:engine
nibName:nil
bundle:nil];
EXPECT_EQ(viewController2.viewId, 0ull);
EXPECT_EQ(viewController2.viewId, 0ll);
EXPECT_EQ(engine.viewController, viewController2);
}
// FVC2 is deallocated but FVC1 is retained.
@@ -695,7 +695,7 @@ TEST_F(FlutterEngineTest, ManageControllersIfInitiatedByEngine) {
engine.viewController = viewController1;
EXPECT_EQ(engine.viewController, viewController1);
EXPECT_EQ(viewController1.viewId, 0ull);
EXPECT_EQ(viewController1.viewId, 0ll);
}
TEST_F(FlutterEngineTest, HandlesTerminationRequest) {

2
engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterEngine_Internal.h
View File

@@ -133,7 +133,7 @@ typedef NS_ENUM(NSInteger, FlutterAppExitResponse) {
/**
* The |FlutterViewController| associated with the given view ID, if any.
*/
- (nullable FlutterViewController*)viewControllerForId:(uint64_t)viewId;
- (nullable FlutterViewController*)viewControllerForId:(int64_t)viewId;
/**
* Informs the engine that the specified view controller's window metrics have changed.

2
engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterPlatformNodeDelegateMacTest.mm
View File

@@ -192,7 +192,7 @@ TEST(FlutterPlatformNodeDelegateMac, CanPerformAction) {
// Set up embedder API mock.
FlutterSemanticsAction called_action;
uint64_t called_id;
int64_t called_id;
engine.embedderAPI.DispatchSemanticsAction = MOCK_ENGINE_PROC(
DispatchSemanticsAction,

4
engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterRenderer.h
View File

@@ -38,12 +38,12 @@
/**
* Called by the engine when the given view's buffers should be swapped.
*/
- (BOOL)present:(uint64_t)viewId texture:(nonnull const FlutterMetalTexture*)texture;
- (BOOL)present:(int64_t)viewId texture:(nonnull const FlutterMetalTexture*)texture;
/**
* Creates a Metal texture for the given view with the given size.
*/
- (FlutterMetalTexture)createTextureForView:(uint64_t)viewId size:(CGSize)size;
- (FlutterMetalTexture)createTextureForView:(int64_t)viewId size:(CGSize)size;
/**
* Populates the texture registry with the provided metalTexture.

8
engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterRenderer.mm
View File

@@ -17,7 +17,7 @@ static FlutterMetalTexture OnGetNextDrawableForDefaultView(FlutterEngine* engine
// TODO(dkwingsmt): This callback only supports single-view, therefore it only
// operates on the default view. To support multi-view, we need a new callback
// that also receives a view ID, or pass the ID via FlutterFrameInfo.
uint64_t viewId = kFlutterDefaultViewId;
int64_t viewId = kFlutterDefaultViewId;
CGSize size = CGSizeMake(frameInfo->size.width, frameInfo->size.height);
return [engine.renderer createTextureForView:viewId size:size];
}
@@ -27,7 +27,7 @@ static bool OnPresentDrawableOfDefaultView(FlutterEngine* engine,
// TODO(dkwingsmt): This callback only supports single-view, therefore it only
// operates on the default view. To support multi-view, we need a new callback
// that also receives a view ID.
uint64_t viewId = kFlutterDefaultViewId;
int64_t viewId = kFlutterDefaultViewId;
return [engine.renderer present:viewId texture:texture];
}
@@ -88,7 +88,7 @@ static bool OnAcquireExternalTexture(FlutterEngine* engine,
#pragma mark - Embedder callback implementations.
- (FlutterMetalTexture)createTextureForView:(uint64_t)viewId size:(CGSize)size {
- (FlutterMetalTexture)createTextureForView:(int64_t)viewId size:(CGSize)size {
FlutterView* view = [_viewProvider viewForId:viewId];
NSAssert(view != nil, @"Can't create texture on a non-existent view 0x%llx.", viewId);
if (view == nil) {
@@ -98,7 +98,7 @@ static bool OnAcquireExternalTexture(FlutterEngine* engine,
return [view.surfaceManager surfaceForSize:size].asFlutterMetalTexture;
}
- (BOOL)present:(uint64_t)viewId texture:(const FlutterMetalTexture*)texture {
- (BOOL)present:(int64_t)viewId texture:(const FlutterMetalTexture*)texture {
FlutterView* view = [_viewProvider viewForId:viewId];
if (view == nil) {
return NO;

4
engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterViewController.mm
View File

@@ -454,7 +454,7 @@ static void CommonInit(FlutterViewController* controller, FlutterEngine* engine)
[_flutterView setBackgroundColor:_backgroundColor];
}
- (uint64_t)viewId {
- (int64_t)viewId {
NSAssert([self attached], @"This view controller is not attched.");
return _viewId;
}
@@ -483,7 +483,7 @@ static void CommonInit(FlutterViewController* controller, FlutterEngine* engine)
return _bridge;
}
- (void)attachToEngine:(nonnull FlutterEngine*)engine withId:(uint64_t)viewId {
- (void)attachToEngine:(nonnull FlutterEngine*)engine withId:(int64_t)viewId {
NSAssert(_engine == nil, @"Already attached to an engine %@.", _engine);
_engine = engine;
_viewId = viewId;

2
engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterViewController_Internal.h
View File

@@ -33,7 +33,7 @@
*
* This method is called by FlutterEngine.
*/
- (void)attachToEngine:(nonnull FlutterEngine*)engine withId:(uint64_t)viewId;
- (void)attachToEngine:(nonnull FlutterEngine*)engine withId:(int64_t)viewId;
/**
* Reset the `engine` and `id` of this controller.

2
engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterViewEngineProvider.mm
View File

@@ -22,7 +22,7 @@
return self;
}
- (nullable FlutterView*)viewForId:(uint64_t)viewId {
- (nullable FlutterView*)viewForId:(int64_t)viewId {
return [_engine viewControllerForId:viewId].flutterView;
}

2
engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterViewEngineProviderTest.mm
View File

@@ -25,7 +25,7 @@ TEST(FlutterViewEngineProviderUnittests, GetViewReturnsTheCorrectView) {
OCMStub([mockEngine viewControllerForId:0])
.ignoringNonObjectArgs()
.andDo(^(NSInvocation* invocation) {
uint64_t viewId;
int64_t viewId;
[invocation getArgument:&viewId atIndex:2];
if (viewId == 0 /* kFlutterDefaultViewId */) {
if (mockFlutterViewController != nil) {

4
engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterViewProvider.h
View File

@@ -4,7 +4,7 @@
#import "flutter/shell/platform/darwin/macos/framework/Source/FlutterView.h"
extern const uint64_t kFlutterDefaultViewId;
extern const int64_t kFlutterDefaultViewId;
/**
* An interface to query FlutterView.
@@ -20,6 +20,6 @@ extern const uint64_t kFlutterDefaultViewId;
*
* Returns nil if the ID is invalid.
*/
- (nullable FlutterView*)viewForId:(uint64_t)id;
- (nullable FlutterView*)viewForId:(int64_t)id;
@end