-
-
Notifications
You must be signed in to change notification settings - Fork 851
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
Unable to create new Image<TPixel> from VB.NET due to the unmanaged constraint #1202
Comments
First thought for a quick win create a static That would unblock most APIs except direct pixel access. |
Whoops, I think this is my fault 😅 See #1122. |
I see this issue has been closed, but I have just downloaded the latest version to try out and the problem still exists. I was using the code in the "Initialising New Images" section of the following page Thank you! |
I would say convert everything to C#, I used to have a program written in VB6, then I converted it to VB.NET and then converted it to C# and I was happy for the performance benefits gained from using C# instead of VB.NET. |
@Bunnywood do you do manual pixel manipulation? If not, non-generic API variants should be fine. |
No it hasn't. It's still open, however I cannot see us being able to fix it without VB.NET adding the language support. |
@Sergio0694 Is there any way we could hide the keyword from the top level API for V3? |
@JimBobSquarePants The sensible answer is: not really, no. The ...That being said, if we reeeeeeally wanted to, yes technically we could hide that keyword from public APIs. Assuming that we just documented those type parameters to always have to respect the There are two possible scenarios I can see for each API here:
Consider this scenario where the
We can then invoke the second from each stub by using Fody.InlineIL: // No top level unmanaged constraint
interface IPixel<TPixel>
where TPixel : struct, IPixel<TPixel>
{
}
// Same here as well, just the struct constraint
class Image<TPixel>
where TPixel : struct, IPixel<TPixel>
{
// Some public API on Image<T>, this is just a stub
public void Foo()
{
if (RuntimeHelpers.IsReferenceOrContainsReferences<TPixel>())
ThrowHelper.ThrowInvalidTypeWithPointersNotSupported(typeof(TPixel));
IL.Emit.Ldarg_0(); // load "this"
IL.Emit.Call(MethodRef.Method(typeof(Image<TPixel>), "FooCore").MakeGenericMethod(typeof(TPixel)));
IL.Emit.Ret();
}
// This is the actual API, where we can use the unmanaged constraint. Note how it's a static
// method taking the same "this" instance as input. Here we're guaranteeing that T will always
// be the same as TPixel, and we'll just use T from now one, and we'll "see" the constraint.
private static void FooCore<T>(Image<T> @this)
where T : unmanaged, IPixel<T>
{
Console.WriteLine($"FooCore<{typeof(T)}>");
}
} So, this works. I should note, there is no reflection involved, there are no additional NuGet dependencies (they're all build-time private assets), and all of this will just get compiled down to "perfect" IL, and resulting in the same good codegen we have today. So, assuming we actually decided to do this, it would in theory be viable and it would solve this issue. |
Wow @Sergio0694 thanks for such a comprehensive response! 😍 I was expecting a few lines at most. Lots to think about. The Fody example is interesting though perhaps unwieldy given the shape of our API. I'd have to do a thorough investigation (delete the unmanaged constrain and see what methods complain 😜) to determine how viable that is. |
I think I have a solution that could work, here's how it could work. Basically we can use I'd be happy to have a much more thorough look through the code and see what exactly would be necessary if you'd like; I'd also be interested in making a PR if the project owners would be alright with it. |
If anything, a solution should be an external utility IMO. Maintaining workarounds for an unmaintained language is hardly worth the efforts. |
I looked through the code and found only 1 instance where Also, VB.NET isn't unmaintained, it's not having new features added in it's own right but is being updated to work with C# features that aren't deemed 'unsafe', which they seemed to have deemed this as. Also, 3.0.0 would be the ideal time to add this since it would have a few source breaking changes specifically to any uses of |
What you are recommending is basically undoing #1122/#1111. Maybe #1122 was a mistake at that time, but spending (even minor) effort to undo it seems like a wrong investment to me when looking at our backlog. There are only 3 upvotes on this issue coming from the community (not core-team members). If there were more people interested in VB, I would vote to do it, but now it doesn't seem to be justified for me. |
Fair enough, I do think it is a shame though, and basically blocks me from using this library (for this project anyway) for quite some time since I have a large amount of pre-existing VB.NET code that is slowly being converted to C#. Also, I don't think most people who have this issue would have upvoted this issue btw (e.g. I hadn't even thought to do it until you mention it now). So it is likely many, many more than 3 (e.g. the issue for |
Obviously, upvotes do not represent absolute truth, but it's useful as a relative metric, together with the activity of the discussion board. Note that this issue is a significant blocker for VB users who want to adapt ImageSharp, which means that I would expect them to land on this issue and complain, but - honestly - there wasn't much traffic since we opened it. This is another indicator that - compared to our general userbase - there aren't many of them. I understand that this is important for you, and we really appreciate that you are offering help, but in the end of the day, it's the core team who ends up maintaining the implemented change. And again, we are not the primary bad cops here. This would be much, much easier if more love was given to VB from Microsoft itself. I posted this dotnet/vblang#300 (comment) ~2.5 years ago, still no reaction from the owners. They are most likely busy working on other stuff; it's a defunded project. |
In C# 11's new feature set unsafer unsafeness, you can now take pointers and sizeof and all the unsafe things to managed types. This means you could change all the Example code that is now legal in C# 11: object o = new object();
object* ptr = &o; //address to o, which is on the stack
ref object refO = ref o; //reference to o
fixed (object* ptr2 = &refO) { } //take address of refO, since the compiler can't be sure if it's on the stack or is an interior reference
int size = sizeof(object); //same as sizeof(IntPtr) for most (or all?) platforms since this is how big an object is (since 'object' is a reference to the actual data) As you can see, you can now do the same things with managed types as you could with unmanaged types before. Many of the above lines will generate warnings though, since the logic is not necessarily valid for managed pointers (e.g. allocating a chunk of memory and then storing object values in it, the GC wouldn't update the references if it moves them), which is why you'd check the type parameter is valid. I understand if you don't want to implement this, but just letting you know it is possible without any change other than constraints and (optionally) checking type parameters and throwing if they're invalid. Hopefully in C# 12 they'll add a way to circumvent type parameter constraints in unsafe mode or with runtime guards, which would solve it for me. If you don't like that solution though, it may actually be possible to generate a seperate Nuget package (called something like SixLabors.ImageSharp.VB) that is ref only (meaning it doesn't contain any actual code), references the real package in a way that doesn't expose it immediately to the project / some other way to make the reference have higher usage priority over the normal package so that it ignores the fact it should have the unmanaged constraint, and just declares everything with struct contraints instead. I've tested that this is indeed possible in theory. |
This is a terrible idea, and also it wouldn't actually work. The point of using As in: static void PublicApi<T>()
where T : struct
{
// You can't use Foo<T> here, no matter what you do
}
class Foo<T>
where T : unmanaged
{
} |
Did you see my second idea (the SixLabors.ImageSharp.VB package)? This is the one I was more thinking would be better. There's not really anything unsafe about it except that it allows you to ignore the unmanaged constraints when you specifically opt-in to do so. It works because adding and removing the unmanaged constraint still produces the same CIL method signature and class name, which is all that's needed. Edit: If you want, give me an example function in the .NET library (for ease of demonstration, and since I can't think of any off the top of my head, I can also just add one that has the unmanaged constraint but it feels a bit like cheating) that uses the unmanaged constraint and I'll send a sample project to demonstrate that you can get around it with a little bit of IL manipulation / creating a ref project (I've actually done it using the ref project, you'd do it with IL manipulation to automate it). Edit 2: sorry for my poor wording in relation to the now-removed MS VB.NET rant section, I'm just upset that MS won't just support ref structs and unmanaged constraint in VB.NET. |
Sample POC setup for SixLabors.ImageSharp.VBMain file: static void Main(string[] args)
{
//allows me to increment a ref readonly as expected
int a = 0;
ref readonly int aRef = ref a;
Class1.Increment(in aRef);
Console.WriteLine(a);
//this doesn't throws as expected:
Class1.PrintBytes(123);
//this throws as expected on method load (hence why it's in a delegate):
try
{
Action action = () => Class1.PrintBytes("abcdef");
action.Invoke();
}
catch (Exception ex)
{
Console.WriteLine(ex.ToString());
}
} You'll need to unzip these, and set up a nuget.config to use them. The project should reference Example nuget.config file (should be on solution level): <configuration>
<packageSources>
<add key="Test Source" value="C:\Path\To\Extracted\Packages\Folder" />
</packageSources>
</configuration> RealLibrary source code (not needed to run sample): namespace RealLibrary
{
public class Class1
{
public static void Increment(ref int i) => i++;
public static unsafe void PrintBytes<T>(ref T value) where T : unmanaged
{
fixed (T* ptr = &value)
{
for (int i = 0; i < sizeof(T); i++)
{
Console.WriteLine($"0x{i:X8}: {((byte*)ptr)[i]}");
}
}
}
}
} RefLibrary source code (not needed to run sample): using System.Runtime.CompilerServices;
[assembly: ReferenceAssembly]
namespace RealLibrary
{
public class Class1
{
public static void Increment(in int i) { throw null; }
public static void PrintBytes<T>(in T value) { throw null; }
}
} The RefLibrary nupkg's nuspec file was modified a bit & the lib folder was renamed to ref. |
Honestly VB has been all but dead in development and it looks like Microsoft just wants everyone to use C#, F#, etc instead as a way to completely kill it. Sadly, I feel like there is no 1 good solution for this issue either (other than trying to get Microsoft to change their minds and update VB to be on par with C# again). |
So, out of curiosity... how many "up votes" would be necessary to get this issue to be seriously considered and resolved? |
You know, I was going to leave it with that comment... but figured I'd leave another for the sake of answering what appears to be a common-thread sort of question across this issue... Let's list some of the reasons why you aren't hearing from "us":
But what about those of us that are blazing the trail and venturing outside of the Windows-only ecosystem? Sure, we might be small in number... but the more we learn and share... the more awareness grows and, maybe, just maybe, the more VB devs will surface needing the same thing. But in the mean time... continue to alienate "us" - because that is certainly forward thinking. It seems odd to me that decisions would be made to alienate an entire pool of potential customers - especially since there is actively a commercial license available for this library. And why haven't you heard from us "before now"? Well, it's kind of simple... the move beyond .NET 4.x for VB developers is a slow one. Several things had to take place before many of us would want to make the move (excluding, well, people like me... I've been on the move since .NET Core 1.0). Windows Forms, yes, that thing... is one of the reasons why the delay. Tooling... in Visual Studio... is another reason - there is little desire by many VB'ers to drop down to the command line to do things. And to suggest that VB is not supported... that's a total farce. What has been stated is "stop moving our cheese"... we feel it isn't necessary to add every conceivable language constraint to the language to, well, get stuff done. A library (API) designer not taking something like this into account... or worse... deciding to expose (and limit said exposure to only be) "unsafe" and/or "unmanaged" API surfaces is, to me, a pretty odd choice. In any case, the choice is yours... and it seems like it should be a pretty simple one as unlike "other languages", what VB does isn't an ongoing moving target. Additionally, as for being not supported... it is the idea that continues to illustrate just how out of touch our overall .NET community is considering... guess what????... changes have been made. They are small in number, but the reality is that movement has taken place... but sure, continue to spout how Microsoft has abandoned it. This is an ignorant statement; and to those that are aware - it's pretty clear. Furthermore, the general idea of telling people "to just switch to C#" is total BS. I suppose I could respond with... well what is stopping all you curly-brace semi-colon diehards from switching to a "cooler" language such as RUST? It's a stupid statement and I really am insulted that you would allow such rhetoric to take place with no sort of "hey, hey... let's all try to get along"... after all, aren't we all supposed to be part of the overall .NET family? Remember... contrary to popular opinion... CLR does not stand for C# Language Runtime. Just saying. And... finally... our numbers might appear small - and the major reason for this is that "we" don't tend to hang out where we clearly aren't welcomed. This is all very disappointing as I totally feel like this library is "the answer" to a problem I'm facing and so looked forward to connecting what I'm doing to your product - raising awareness to your product to a potential host of other VB developers. In any case, great approach to what you are doing by only taking a dependency on .NET... truly mean that. It's exactly what I was hoping to find. It's just too bad that there is a total resistance to welcoming part of your .NET family to join in the awesomeness that is. Sincerely... A very disappointed VB developer. |
The official VB language strategy is here: https://learn.microsoft.com/en-us/dotnet/visual-basic/getting-started/strategy. No, it is not dead, but much as is the case with .NET Framework the language is effectively frozen and not getting new work or features outside absolutely critical stuff. What this basically boils down to is that newer features or APIs may be beyond the reach of VB customers and not every one of those features is esoteric. Things like Using
Third party community maintained libraries are not the CLR. Even the CLR makes decisions on whether new features should be applicable to all languages or just some languages. Many new features are what you would call The same goes with the BCL where API review makes decisions around whether some APIs should be general purpose or not. We do try to keep most APIs general purpose and typically do things like expose both It's ultimately the decision of the individual library whether or not to support a given language. However, given that this is open source and library maintainers may or may not be intricately familiar with VB and its limits/expectations; a better approach might be to state the issue you have, identify some of the blockers, and suggest possible alternatives that would allow that support to exist without greatly shifting the balance of maintenance/complexity/etc. Being demanding that support should exist doesn't help anyone :) |
Was dynamic s = default(MyStruct);
C.Method(s);
class C { public static void Method<T>(T t) where T : unmanaged => Console.WriteLine(typeof(T)); }
struct MyStruct { public string S; } |
Actually it might have been introduced for that reason only, @Sergio0694 do you remember the details? If this is the case IMO it is OK to have a community PR switching it back to |
@DualBrain note that if you are willing to create a small layer in C# in your app, this is really trivial to workaround. |
@MichalStrehovsky @antonfirsov @DualBrain I get that you are disappointed, but you really need to appreciate that the number of active contributors to this library is VERY small. We are not supported by Microsoft, nor are responsible for CLR support across various languages. We're just trying to write something that is fast and reasonable to maintain. |
Ah yeah. Then time to close it I guess. |
Do the users have to write their own static abstracts and/or call them? VB can still consume types that have static abstracts, it just cannot call the static abstracts themselves or make types that implement them. I.e. this works fine and my expectation is that this would be the primary use case for users of ImageSharp: public interface IFoo { static abstract void Frob(); }
public class C : IFoo { public static void Frob() => Console.WriteLine("Hello"); }
public class Gen<T> where T : IFoo { public static void Frob() => T.Frob(); } Module Program
Sub Main(args As String())
Gen(Of C).Frob()
End Sub
End Module Maybe not everything would be expressible but that's fine. If static abstracts are a blocker or But if it's fixable and someone is willing to contribute it and the maintainers accept it, it would be overall goodness. A lot of |
@MichalStrehovsky If VB users can consume The V4 branch targets .NET 8/ C#12 so if we do not need the unmanaged keyword anymore and if VB users are happy to use existing types (@DualBrain) then this change may be unblocked. However… @tannergooding mentioned |
While you can't store spans in locals, you can still read from them, write to them, convert to/from array, etc. There was a discussion about this the other day in allow-unsafe-blocks, I can link you to it in a little while if you want, but basically: you can do a number of useful things with them in VB.NET, although not everything (mainly you can use them transitively) SpanAction might not work well though if you're using something like that |
Thanks @hamarb123 we may be able to support VB in V4 then if I understand everything correctly: |
Thanks for continuing the conversation... in my particular use case, I'm mainly interested in having a viable cross-platform answer to loading (maybe saving?) image formats in a similar manner to System.Drawing and, at some point, be able to at least read the pixel data. If modifying the pixel data is directly, I'd like to see some method that can take raw pixel (RGBA array) as a construction/conversion mechanism. Specifically, what I'm needing is the ability to open a file (PNG) and read the pixel data into another array that will then be managed outside of ImageSharp. It'd be nice to be able to then take this array of data and (potentially) save it as a file (PNG). Any sort of customization features or additional features beyond what System.Drawing provide are certainly beyond any scope of what I personally need. Don't get me wrong, it might be nice... but I haven't personally come across anyone in the VB community at this point that would need custom types and as for accessing a span, the overhead of converting a span to array (which I believe is supported in VB - but would need to double check) and working against that resulting array, IMHO, should be fine. In other words, it's the cross-platform feature with (hopefully) similar functionality to System.Drawing that I feel is of the most interest to the VB developer. And given that this project is able to target any platform supported by .NET makes this very attractive. And this is where I would be focusing any of my, let's call it, evangelism re: this project to the VB community. More is always great, but really focused on this core scenario - everything ImageSharp can do beyond this is a welcomed bonus; but we can all should be able to agree that some things are designed to take advantage of advanced features in the name of performance gains requiring specific language features of C# is OK. And although VB can certainly be written in such a way that can be performant... raw (unsafe, etc.) access will always be better. But I don't think VB is really (at least not always) about raw performance metrics; so any overhead incurred due to using VB (span vs array) is understood. Again, thanks for continuing the conversation - I really do appreciate it. |
I believe this to be the case; and if it isn't it would certainly fall under what has been committed to for VB's "consumption-only approach* portion of the "strategy" (I suppose I should also add... by Microsoft). |
Hi DualBrain, |
There is a user on Stack Overflow trying to create a new image using VB.NET. https://stackoverflow.com/questions/61839616/imagesharp-draw-image-on-image-in-vb-net
Image sharp has an issue at the moment where its not possible to new up a new
Image<TPixel>
or call any of ourTPixel
based APIs from VB.NET as all ourTPixel
APIs have theunmanaged
constraint and that is not (and apparently will not be supported in VB.NET)We need to come up with a solution to this.
Related
dotnet/vblang#300
https://stackoverflow.com/questions/61839616/imagesharp-draw-image-on-image-in-vb-net
The text was updated successfully, but these errors were encountered: