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

Functions to move to dart:core from dart:ui #25217

Open
1 of 4 tasks
Hixie opened this issue Dec 10, 2015 · 18 comments
Open
1 of 4 tasks

Functions to move to dart:core from dart:ui #25217

Hixie opened this issue Dec 10, 2015 · 18 comments
Labels
area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. core-2 type-enhancement A request for a change that isn't a bug

Comments

@Hixie
Copy link
Contributor

Hixie commented Dec 10, 2015

These are functions and types that we have in dart:ui (Flutter's core library) that really should just be in dart:core:

@sethladd
Copy link
Contributor

cc @floitschG

@sethladd sethladd added area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. enhancement labels Dec 10, 2015
@lrhn
Copy link
Member

lrhn commented Dec 12, 2015

The lerpDouble function could be put on double (or num) as

double interpolate(num a, num b) => a + (b - a) * this.toDouble();

(it should return double, not num).

I'm more concerned about the hash functions.
The 373 (* 37 + x)* is a classic, so we can at least say that we are no worse than everybody else, but it's not great. It's quick, though.
We have other hash implementations spread around the source code that we could centralize and expose.

@Hixie
Copy link
Contributor Author

Hixie commented Dec 12, 2015

Just to be clear, the algorithms used in the Flutter hash functions today are horrible; we'd love to have better ones if you take these functions over. We're planning on changing the implementations of the ones we use anyway (flutter/flutter#859). It's the interface we're interested in moving over to core Dart, not the precise implementations.

If lerpDouble made it over that would be great. We currently have lerp functions on many of our classes (Point, Rect, Color, Border, BoxDecoration, etc) but since there's no function on "num" for it, we have to call this function instead when we need to lerp a double. Calling it "interpolate" would be fine, we'd change our functions to match (though note that "lerp" is a pretty widely used term of art).

@kasperpeulen
Copy link

I often see people use those hash functions in Dart libraries:

https://github.com/google/quiver-dart/blob/master/lib/src/core/hash.dart

@floitschG
Copy link
Contributor

We discussed the lerp function, and it seems simpler if this stays a helper function. It's a common operation in drawing/3D, but is otherwise not much used. Maybe we can get it through extension methods (no promises).

@Hixie
Copy link
Contributor Author

Hixie commented Nov 29, 2018

An update from 2018:

  • Add typedef void VoidCallback(); to dart:core #27791 covers moving VoidCallback to dart:core also.
  • We would still love these four members moved. It would help with code that uses the Foundation library in Flutter but wants to run in non-Flutter environments like servers.
  • For hashValues and hashList we still care about only the interface, and are happy with any implementation, so long as it is performant (when called and in terms of giving good hashing behaviour).
  • For lerpDouble, having it be double.lerp would still be preferable IMHO but if it's just lerpDouble that's fine too. We now use lerp extensively in our API so would very much prefer that it not be renamed to interpolate.

@matanlurey
Copy link
Contributor

As of 2018, void Function() is the same as VoidCallback 🤷‍♂️

@dnfield
Copy link
Contributor

dnfield commented Jan 22, 2019

For the hashing functions,see #11617

@dnfield
Copy link
Contributor

dnfield commented Jan 22, 2019

Should we just mark VoidCallback as deprecated and convert all instances of it to void Function()?

@Hixie
Copy link
Contributor Author

Hixie commented Jan 22, 2019

Why would we do that?

@dnfield
Copy link
Contributor

dnfield commented Jan 22, 2019

It would achieve our goal of eliminating a dart:ui dependency where we don't want it, assuming we can live with having a void Function() signature instead of VoidCallback.

@kevmoo
Copy link
Member

kevmoo commented Jan 22, 2019

@lrhn had spent some time thinking about hashCode somewhere in dart: – I think

@dnfield
Copy link
Contributor

dnfield commented Jan 22, 2019

@lrhn had spent some time thinking about hashCode somewhere in dart: – I think

@kevmoo that's the issue I linked to above, #11617

@Hixie
Copy link
Contributor Author

Hixie commented Jan 22, 2019

@dnfield I think using VoidCallback has more value than not depending on dart:ui. I agree it would be ideal if dart:core declared VoidCallback, but it looks like that's not on the cards.

@Hixie
Copy link
Contributor Author

Hixie commented Jul 12, 2023

Update from 2023:

  • The hash functions are now in dart:core and deprecated in dart:ui, hooray!
  • lerpDouble would still be great as double.lerp for consistency with the many, many other static lerp functions in Flutter's framework.
  • It would be great if clampDouble could also be moved somehow. That's a version of double.clamp that's much more efficient than double.clamp. Alternatively, double.clamp could be optimized so as to make clampDouble unnecessary. See make sure that int.clamp and double.clamp are optimised in AOT #46879.
  • VoidCallback would be great to add to dart:core also. It's in a dart:* library now, dart:html, but we can't depend on that from dart:ui.

@kevmoo
Copy link
Member

kevmoo commented Jul 13, 2023

VoidCallback is trivial to define at the use site. void Function() – right?

@Hixie
Copy link
Contributor Author

Hixie commented Jul 13, 2023

Well it would be typedef VoidCallback = void Function(); since types with spaces and parentheses in them are something we try to avoid but the idea is to just define it once and not worry about having to redeclare it everywhere. That way you can be confident that you're really talking about the same type, it cross-links in API docs, etc. As noted earlier in this bug, using VoidCallback has more value than not depending on dart:ui, but it would be great to have both.

@natebosch
Copy link
Member

After discussion I don't think there are any signatures/behaviors that make sense to land in the core libraries. For double.clamp and double.lerp the core library maintainers are aligned on waiting to see whether we are able to land static extension methods as the best solution here. There are questions about whether Flutter authors prefer static extensions over the status quo, but from the Dart core library standpoint we're confident with pushing in that direction.

For VoidCallback we're aligned on messaging a preference for void Function() and don't have plans to add the typedef to the core libraries.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. core-2 type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

No branches or pull requests

9 participants