Add loading of in-memory resources. #578

Bindernews · 2020-07-08T23:14:16Z

I do NOT consider this PR merge-ready yet. It needs testing on Mac and iOS, and support for LoadBitmap needs to be added to the Canvas backend, and maybe the Cairo, AGG and Lice backends. That being said, I'm putting it up for review.

Major changes:

New overloads for LoadBitmap(), LoadSVG(), and LoadFont() to enable loading resources directly from memory.
xxd.py in the Scripts directory for generating C++ source/header files from binary files. Ideally this would become part of the build process for users who want to use this feature.

AlexHarker · 2020-07-09T00:35:25Z

Thanks - this is looking pretty good, although a few things to sort, some testing needed, and some possibilities for reducing duplication a little I think.

A few things so far:

Web builds are failing CI, so there's something wrong there - the error from the build logs online is:
../../../IGraphics/Platforms/IGraphicsWeb.cpp:104:32: error: member initializer 'mSystem' does not name a non-static data member or base class
: Font(fontName, fontStyle), mSystem(false)
Mac platform font loading assumes that the data is persistent - that is to say that you directly wrap the data in a provider, and then that provider hangs around, and may be re-used, at which point if the original pointer is no longer valid something will go wrong and quite possibly crash - we should probably copy and provide a release function
I think the windows platform font loading has scope for reducing duplication because where we load from ID we get pFontMem and resSize which could just be forwarded to the new method

AlexHarker · 2020-07-09T00:38:38Z

BTW - we should add some tests for this in IGraphicsTest, which will make life easier in the long run.

Bindernews · 2020-07-09T02:45:19Z

I think I know what I did wrong regarding the web stuff, the Mac fix should be pretty easy as well, and I know what you're talking about regarding the Windows code deduplication.

As for the testing, are you talking about stuff in IGraphics/Controls/Test? If so, are we using a specific testing framework, or just running the code?

olilarkin · 2020-07-09T07:08:02Z

Currently the tests folder contains projects, no unit tests, but I’ve been thinking about adding some tests with catch2

AlexHarker · 2020-07-09T08:02:03Z

Yes - currently we make custom controls for testing (by the way @oli - right now the test controls are in IGraphics/Controls/Test, but wouldn't it make more sense to has them in Tests/Controls or somewhere like that?

Bindernews · 2020-07-09T18:01:32Z

I'm noticing a lot of Windows API calls that could be easier to manage with a custom deleter. Something like this:

template<typename T>
using wrapped_ptr = std::unique_ptr<T, std::function<void(T*)>>;
...
HANDLE file = CreateFile(...);
if (file)
{
  auto pFile = wrapped_ptr(&file, [](HANDLE* h) { if (*h) { CloseHandle(*h); } });
  // ... CloseHandle will automatically be called when pFile goes out of scope.
}

This might make managing some of the Windows API code easier and less likely to leak. This is applicable specifically to the font code, but also to other parts of the Windows API and probably other platform APIs as well.

Bindernews · 2020-07-09T19:33:36Z

Okay, I'm just going to get emscripten setup so I can actually compile stuff to make sure it works. XD

… to comment out GL.newRenderingFrameStarted() in the main loop.

Bindernews · 2020-07-10T17:50:40Z

Okay! I think this is finally ready for a real review. It's working on NanoVG, Skia, and Canvas, as well as Windows and probably Mac, though memory font loading on Mac needs to be confirmed. @olilarkin @AlexHarker Can I get some feedback please?

Also note: I have a Testing project in the Examples directory, which I was using for testing web builds. I'll make one more commit before we merge to get rid of that, but I'm leaving it for now so you can see Canvas working.

…terface.

…mory-loading

olilarkin · 2020-07-13T11:40:49Z

How do you envisage dealing with multi-res bitmaps? Currently IGraphics::LoadBitmap() will find @2x.png , @3x.png , finding those files on disk, so you only need a single Load bitmap line. I was experimenting with your code, having used bin2c.py to convert the knob and knob@2x.png from the IPlugControls example, and I had to make a slight modification in order to get the multi-res bitmap to load properly on different screens.

I think it would be great if we could make bin2c.py and your new IGraphics::LoadBitmap() deal with multi-res pngs automatically, so you can #include a single file and call LoadBitmap once.

diff --git a/Examples/IPlugEffect/IPlugEffect.cpp b/Examples/IPlugEffect/IPlugEffect.cpp
index daf7a81f3..7f9f6b58d 100644
--- a/Examples/IPlugEffect/IPlugEffect.cpp
+++ b/Examples/IPlugEffect/IPlugEffect.cpp
@@ -1,6 +1,8 @@
 #include "IPlugEffect.h"
 #include "IPlug_include_in_plug_src.h"
 #include "IControls.h"
+#include "knob.h"
+#include "knob2x.h"
 
 IPlugEffect::IPlugEffect(const InstanceInfo& info)
 : Plugin(info, MakeConfig(kNumParams, kNumPresets))
@@ -16,9 +18,13 @@ IPlugEffect::IPlugEffect(const InstanceInfo& info)
     pGraphics->AttachCornerResizer(EUIResizerMode::Scale, false);
     pGraphics->AttachPanelBackground(COLOR_GRAY);
     pGraphics->LoadFont("Roboto-Regular", ROBOTO_FN);
+    
+    // maybe we should have a way of loading multi-res bitmaps from memory in a single line, like we do for disk based resources
+    IBitmap bmp = pGraphics->LoadBitmap("knob.png", knob, knob_length, 60);
+    IBitmap bmp2 = pGraphics->LoadBitmap("knob.png", knob2x, knob2x_length, 60, false, 2);
+
     const IRECT b = pGraphics->GetBounds();
-    pGraphics->AttachControl(new ITextControl(b.GetMidVPadded(50), "Hello iPlug 2!", IText(50)));
-    pGraphics->AttachControl(new IVKnobControl(b.GetCentredInside(100).GetVShifted(-100), kGain));
+    pGraphics->AttachControl(new IBKnobControl(b.GetCentredInside(100), bmp, kGain));
   };
 #endif
 }
diff --git a/IGraphics/IGraphics.cpp b/IGraphics/IGraphics.cpp
index e86f2f542..97e5cb662 100644
--- a/IGraphics/IGraphics.cpp
+++ b/IGraphics/IGraphics.cpp
@@ -1756,7 +1756,7 @@ IBitmap IGraphics::LoadBitmap(const char *name, const void *pData, int dataSize,
     // It's definitely not loaded, so load it with scale = 1.
     if (!pAPIBitmap)
     {
-      loadedBitmap = std::unique_ptr<APIBitmap>(LoadAPIBitmap(name, pData, dataSize, 1));
+      loadedBitmap = std::unique_ptr<APIBitmap>(LoadAPIBitmap(name, pData, dataSize, targetScale));
       pAPIBitmap= loadedBitmap.get();
     }

olilarkin · 2020-07-13T11:44:54Z

Also, I wonder if putting everything in a header file is best, or if the tool should make header + .c/pp file ? In my example above, every time I want to rebuild the plugin the compiler will have to process the included header files again i think, slowing build times if they included a lot of data

Bindernews · 2020-07-13T14:51:38Z

I'm don't really think that in-memory loading should decide what scale to load at. It should take a block of memory and load that image, regardless of scale. We could easily make a helper method that takes an array of ImageData objects or something and loads the correct one from there, but we shouldn't make the simple use-case harder for the sake of the complex use-case.

Secondly, my original system had some more support for that, but I removed it to be more compatible with bin2c. I can easily copy over some of that code and generate a header file, but again we have the problem of a more complex tool making the simple use-case harder.

I'd say that if you want a function that loads based on the current scale, that should be a separate function. I can add it to the PR today. I can also add the option to generate a header file with the cpp file. What we probably shouldn't do is over-complicate the resource generator tool. One resource file -> one .cpp file. If they want to make an array of resource files, that's on them.

AlexHarker · 2020-07-13T20:10:25Z

I'm with @olilarkin - currently when you use one filename it looks for multiple scales of the same filename so it is seamless to the developer - so to my mind the resource generator should make the array and when you load it IGraphics knows which scale is best - otherwise it's more work to use this method than the normal ones.

Bindernews · 2020-07-13T20:53:42Z

Okay, I can put my multiple-file code back in, but how will it be specified? I'm thinking a CLI flag would be best, maybe -m or --multiple which will then search for files with the @<int> suffix and turn the whole thing into an array, even if there's only one file.

That way if the user is including a font, SVG, or some other binary blob, it remains simple. If they want the file-array, it should be opt-in.

olilarkin · 2020-07-13T21:35:58Z

that sounds fine

Bindernews · 2020-07-13T21:44:30Z

Should the script automatically pick up multiple resolutions of the file, or should the user have to specify them manually?

olilarkin · 2020-07-13T22:44:40Z

It would be great to do it automatically of course!

…

On Mon, Jul 13, 2020 at 10:44 PM Binder News ***@***.***> wrote: Should the script automatically pick up multiple resolutions of the file, or should the user have to specify them manually? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#578 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAFACLTSMTFR6XNIHN3AYTDR3N54ZANCNFSM4OVBFHWQ> .

…iple files.

Bindernews · 2020-07-13T23:01:26Z

Okay, I have what I think is a pretty nice balance. First, both the source file and header file are now required arguments. Technically the script still supports not generating a header, but I'm not going to make it an option from the CLI as I think it's more confusing. Almost everyone will want header files, and if they don't they can just delete it or something.

Now, files with the scaling suffix are added with the -s or --scaled flag. Using the -s flag will perform a search for scaled files with the same name and include those files. If you want an array you must specify the -a or --array flag with the array name. This is because I expect people might be generating multiple resource files and I don't want name collisions.

In addition to the scaled files, you can now specify multiple files individually on the command line. Each file is specified as a pair of arguments <input_file> <c_array_name>. So you can get multiple files with or without the -s flag, and if you're just trying to convert a single file it's only 4 arguments (cpp_file h_file input_file c_name).

Bindernews · 2020-07-13T23:02:25Z

Oh, and compression is still a feature and is applied to all files if specified. So the simple use-case is still easy, and adding scaled resources and the array feature are just an extra flag each.

olilarkin · 2020-07-14T11:12:57Z

IPlugControls example seems to fail loading svgs. If statements around fseek in IGraphics::LoadResource() seem wrong

    //if (!fseek(fd, 0, SEEK_END))
    if (fseek(fd, 0, SEEK_END))
    {
      fclose(fd);
      return result;
    }

    long size = ftell(fd);
    //if (!fseek(fd, 0, SEEK_SET))
    if (fseek(fd, 0, SEEK_SET))
    {
      fclose(fd);
      return result;

fixes it

Bindernews · 2020-07-14T13:40:38Z

I thought I tested that, oops. I always get confused because some things return 0 or an error code, and other things return 0 or 1 as a boolean. It's quite annoying. I'll make the change and confirm the fix.

olilarkin · 2020-07-14T21:27:54Z

Is there a way to IGraphics::LoadBitmap() in one line now for a multi-res bitmap? I didn't work it out if so. @AlexHarker i think this is close to mergeable now, but missing AGG, LICE & CAIRO impl.

Bindernews · 2020-07-14T22:51:46Z

There is not yet a one-liner for loading multi-res bitmaps, because I have some questions on how to do so.

Do I need to store each resolution in the storage? If so then I think I have some name manipulation to do.
Should I define resource_t globally and assume that all resource_t definitions will be the same? (They should be). Alternately I could use a template<typename R> and make the array version take any type that has name, data, and size properties. The downside there is that it becomes a template method, with its implementation in the header.

As for AGG, LICE, and CAIRO, are you sure you want them? If people are really using them, I'll get them working, but I don't want to spend the time getting them to work if we're going to drop support for a backend soon.

olilarkin · 2020-07-15T23:23:59Z

need to think about those questions.

if you comment # WEB_LDFLAGS += $(NANOVG_LDFLAGS)

then you don't get the WebGL error when using canvas

Bindernews · 2020-07-20T16:38:26Z

Any update on this? I'd like to continue working and get this merged, but I need some decisions.

AlexHarker · 2020-07-20T18:05:06Z

Can get back to this later in more detail, but the first question is do you understand what happens with multiple resolution bitmaps if they are on disk? One of the issues here is that from disk you can load the one you need right now and then later we find the one we need when it changes - that would be the best kind of solution, but obviously there is no such thing as a "path" for memory loading - might need some back and forth on that one.

Bindernews · 2020-07-20T20:58:39Z

That's why the resource_t struct includes a name field, so that we can fake a path.

Bindernews · 2020-07-25T17:06:25Z

Just noticed that this would fix issue #210 .

AlexHarker · 2020-07-25T19:05:45Z

Sadly I think #210 is about having a modifiable bitmap in memory that can be edited and then drawn - I don't think you'd want to have to load that resource in each time.

I actually got quite far with that issue 7 months ago (https://github.com/iPlug2/iPlug2/tree/IRawBitmap), but there were some downsides and it isn't currently merged.

Bindernews · 2020-08-03T19:31:45Z

Let's clarify things so I can get this done and it can be merged:

Need AGG, LICE, and CAIRO implementations. As long as we're saying we're supporting those backends, I'll make it work.
A function that takes an array of resource_t objects and loads them into the cache. This way you can have a one-liner that loads multiple resolutions of bitmaps.

Is that everything? Because I'd really like to get this merged and be able to use it from master.

olilarkin · 2020-08-05T17:53:38Z

I would be happy to merge without 1), as long as those backends compile with no-ops

is great to have but can do it at a later stage if you want?

olilarkin · 2020-08-05T20:51:07Z

merged in b8c4663, may make some extra issues for single-line loading etc

Implementing loading resources from memory.

9c843c5

This should fix the Web/Mac issues, and hopefully reuse code on Windows.

1009a38

Bindernews added 5 commits July 9, 2020 15:43

Merge https://github.com/iPlug2/iPlug2 into memory-loading

4136784

Fixed a typo, also need to clone this on my other machine for testing.

bb78820

More progress on getting memory loading to work on Canvas

6541d13

Fix bitmap loading in NanoVG

00bc875

Fix memory bitmap loading in Canvas API. NOTE: for some reason I have…

470ef62

… to comment out GL.newRenderingFrameStarted() in the main loop.

Bindernews added 2 commits July 12, 2020 18:32

Revamp xxd.py into bin2c.py which is much faster and has a simpler in…

1b0e93b

…terface.

Merge branch 'memory-loading' of github.com:Bindernews/iPlug2 into me…

6904c5b

…mory-loading

Improve bin2c.py to support loading scaled image resolutions and mult…

c3fc784

…iple files.

Oops. I mixed up error codes and boolean return values.

82e60c9

olilarkin added a commit that referenced this pull request Aug 5, 2020

Merge Bindernews: Add loading of in-memory resources. #578

7628e75

olilarkin mentioned this pull request Aug 5, 2020

Merge Bindernews: Add loading of in-memory resources. #578 #598

Closed

olilarkin added a commit that referenced this pull request Aug 5, 2020

Merge Bindernews: Add loading of in-memory resources. #578

b8c4663

olilarkin closed this Aug 5, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add loading of in-memory resources. #578

Add loading of in-memory resources. #578

Bindernews commented Jul 8, 2020

AlexHarker commented Jul 9, 2020 •

edited

AlexHarker commented Jul 9, 2020

Bindernews commented Jul 9, 2020

olilarkin commented Jul 9, 2020

AlexHarker commented Jul 9, 2020

Bindernews commented Jul 9, 2020

Bindernews commented Jul 9, 2020

Bindernews commented Jul 10, 2020

olilarkin commented Jul 13, 2020

olilarkin commented Jul 13, 2020

Bindernews commented Jul 13, 2020

AlexHarker commented Jul 13, 2020

Bindernews commented Jul 13, 2020

olilarkin commented Jul 13, 2020

Bindernews commented Jul 13, 2020

olilarkin commented Jul 13, 2020 via email

Bindernews commented Jul 13, 2020

Bindernews commented Jul 13, 2020 •

edited

olilarkin commented Jul 14, 2020 •

edited

Bindernews commented Jul 14, 2020

olilarkin commented Jul 14, 2020 •

edited

Bindernews commented Jul 14, 2020 •

edited

olilarkin commented Jul 15, 2020

Bindernews commented Jul 20, 2020

AlexHarker commented Jul 20, 2020

Bindernews commented Jul 20, 2020

Bindernews commented Jul 25, 2020

AlexHarker commented Jul 25, 2020

Bindernews commented Aug 3, 2020

olilarkin commented Aug 5, 2020

olilarkin commented Aug 5, 2020

Add loading of in-memory resources. #578

Add loading of in-memory resources. #578

Conversation

Bindernews commented Jul 8, 2020

AlexHarker commented Jul 9, 2020 • edited

AlexHarker commented Jul 9, 2020

Bindernews commented Jul 9, 2020

olilarkin commented Jul 9, 2020

AlexHarker commented Jul 9, 2020

Bindernews commented Jul 9, 2020

Bindernews commented Jul 9, 2020

Bindernews commented Jul 10, 2020

olilarkin commented Jul 13, 2020

olilarkin commented Jul 13, 2020

Bindernews commented Jul 13, 2020

AlexHarker commented Jul 13, 2020

Bindernews commented Jul 13, 2020

olilarkin commented Jul 13, 2020

Bindernews commented Jul 13, 2020

olilarkin commented Jul 13, 2020 via email

Bindernews commented Jul 13, 2020

Bindernews commented Jul 13, 2020 • edited

olilarkin commented Jul 14, 2020 • edited

Bindernews commented Jul 14, 2020

olilarkin commented Jul 14, 2020 • edited

Bindernews commented Jul 14, 2020 • edited

olilarkin commented Jul 15, 2020

Bindernews commented Jul 20, 2020

AlexHarker commented Jul 20, 2020

Bindernews commented Jul 20, 2020

Bindernews commented Jul 25, 2020

AlexHarker commented Jul 25, 2020

Bindernews commented Aug 3, 2020

olilarkin commented Aug 5, 2020

olilarkin commented Aug 5, 2020

AlexHarker commented Jul 9, 2020 •

edited

Bindernews commented Jul 13, 2020 •

edited

olilarkin commented Jul 14, 2020 •

edited

olilarkin commented Jul 14, 2020 •

edited

Bindernews commented Jul 14, 2020 •

edited