Skip to content

Commit

Permalink
Outofbandcache
Browse files Browse the repository at this point in the history
Ok, got this passing testes, which I guess is a step in the right direction.

Before changing unique pointers to rcp, we could only use an image once, as soon as we assigned a new image, the old image would get garbage collected. (this is the actual problem we are trying to solve)

This "solves" the problem and  allows us to add images to a cache in wasm, and then cycle through the cache, swapping out the render image to be displayed for a given fileAsset. We can replace images & fonts on the fly this way.

Problems:
1. overwriting ref & unref in RenderImage, shows that is being reffed and un-reffed constantly (when viewing the attached file with the viewer). Is that expected? doing something similar for Font does not show up the same. I suspect this is related to how we use images when it comes to rendering, just figured i'd check
2. We dont seem to "leak" anything as detectable by our tests, which is good, but our wasm examples show that neither our fonts nor our render images get collected. presumably thats because we would need to manually tell wasm via js that we want to let go of an image?

Also @csmartdalton @umberto-sonnino & @luigi-rosso  it would be great to see what else is concerning.

example file

[just_a_box_with_a_tree.riv.zip](https://github.com/rive-app/rive/files/13042414/just_a_box_with_a_tree.riv.zip)

Diffs=
1506b069c Outofbandcache (#6049)

Co-authored-by: Maxwell Talbot <[email protected]>
  • Loading branch information
mjtalbot and mjtalbot committed Oct 26, 2023
1 parent 84d4f77 commit 04de471
Show file tree
Hide file tree
Showing 18 changed files with 35 additions and 34 deletions.
2 changes: 1 addition & 1 deletion .rive_head
Original file line number Diff line number Diff line change
@@ -1 +1 @@
40f9d91ac8063448c357682ea97d5abbfe71ce1f
1506b069c01a049034052e5de1cbc6a9cf029c2e
2 changes: 1 addition & 1 deletion cg_renderer/include/cg_factory.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ class CGFactory : public Factory

std::unique_ptr<RenderPaint> makeRenderPaint() override;

std::unique_ptr<RenderImage> decodeImage(Span<const uint8_t>) override;
rcp<RenderImage> decodeImage(Span<const uint8_t>) override;
};

} // namespace rive
Expand Down
4 changes: 2 additions & 2 deletions cg_renderer/src/cg_factory.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -518,9 +518,9 @@ std::unique_ptr<RenderPaint> CGFactory::makeRenderPaint()
return std::make_unique<CGRenderPaint>();
}

std::unique_ptr<RenderImage> CGFactory::decodeImage(Span<const uint8_t> encoded)
rcp<RenderImage> CGFactory::decodeImage(Span<const uint8_t> encoded)
{
return std::make_unique<CGRenderImage>(encoded);
return make_rcp<CGRenderImage>(encoded);
}

AutoCF<CGImageRef> CGRenderer::FlipCGImageInY(AutoCF<CGImageRef> image)
Expand Down
4 changes: 2 additions & 2 deletions include/rive/assets/image_asset.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ namespace rive
class ImageAsset : public ImageAssetBase
{
private:
std::unique_ptr<RenderImage> m_RenderImage;
rcp<RenderImage> m_RenderImage;

public:
ImageAsset() {}
Expand All @@ -22,7 +22,7 @@ class ImageAsset : public ImageAssetBase
bool decode(Span<const uint8_t>, Factory*) override;
std::string fileExtension() const override;
RenderImage* renderImage() const { return m_RenderImage.get(); }
void renderImage(std::unique_ptr<RenderImage> renderImage);
void renderImage(rcp<RenderImage> renderImage);
};
} // namespace rive

Expand Down
2 changes: 1 addition & 1 deletion include/rive/factory.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ class Factory

virtual std::unique_ptr<RenderPaint> makeRenderPaint() = 0;

virtual std::unique_ptr<RenderImage> decodeImage(Span<const uint8_t>) = 0;
virtual rcp<RenderImage> decodeImage(Span<const uint8_t>) = 0;

virtual rcp<Font> decodeFont(Span<const uint8_t>);

Expand Down
1 change: 1 addition & 0 deletions include/rive/refcnt.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,7 @@ template <typename T> class rcp
return *this;
}

// move assignment operator
rcp<T>& operator=(rcp<T>&& other)
{
this->reset(other.release());
Expand Down
2 changes: 1 addition & 1 deletion include/rive/renderer.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ class RenderPaint
virtual void invalidateStroke() = 0;
};

class RenderImage
class RenderImage : public RefCnt<RenderImage>
{
protected:
int m_Width = 0;
Expand Down
2 changes: 1 addition & 1 deletion include/utils/no_op_factory.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ class NoOpFactory : public Factory

std::unique_ptr<RenderPaint> makeRenderPaint() override;

std::unique_ptr<RenderImage> decodeImage(Span<const uint8_t>) override;
rcp<RenderImage> decodeImage(Span<const uint8_t>) override;
};
} // namespace rive
#endif
2 changes: 1 addition & 1 deletion skia/renderer/include/skia_factory.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ class SkiaFactory : public Factory

std::unique_ptr<RenderPaint> makeRenderPaint() override;

std::unique_ptr<RenderImage> decodeImage(Span<const uint8_t>) override;
rcp<RenderImage> decodeImage(Span<const uint8_t>) override;

//
// New virtual for access the platform's codecs
Expand Down
4 changes: 2 additions & 2 deletions skia/renderer/src/skia_factory.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -282,7 +282,7 @@ std::unique_ptr<RenderPaint> SkiaFactory::makeRenderPaint()
return std::make_unique<SkiaRenderPaint>();
}

std::unique_ptr<RenderImage> SkiaFactory::decodeImage(Span<const uint8_t> encoded)
rcp<RenderImage> SkiaFactory::decodeImage(Span<const uint8_t> encoded)
{
sk_sp<SkData> data = SkData::MakeWithCopy(encoded.data(), encoded.size());
auto image = SkImage::MakeFromEncoded(data);
Expand All @@ -309,5 +309,5 @@ std::unique_ptr<RenderImage> SkiaFactory::decodeImage(Span<const uint8_t> encode
}
}

return image ? std::make_unique<SkiaRenderImage>(std::move(image)) : nullptr;
return image ? make_rcp<SkiaRenderImage>(std::move(image)) : nullptr;
}
2 changes: 1 addition & 1 deletion src/assets/font_asset.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ using namespace rive;

bool FontAsset::decode(Span<const uint8_t> data, Factory* factory)
{
m_font = factory->decodeFont(data);
font(factory->decodeFont(data));
return m_font != nullptr;
}
std::string FontAsset::fileExtension() const { return "ttf"; }
Expand Down
4 changes: 2 additions & 2 deletions src/assets/image_asset.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,11 @@ bool ImageAsset::decode(Span<const uint8_t> data, Factory* factory)
#ifdef TESTING
decodedByteSize = data.size();
#endif
m_RenderImage = factory->decodeImage(data);
renderImage(factory->decodeImage(data));
return m_RenderImage != nullptr;
}

void ImageAsset::renderImage(std::unique_ptr<RenderImage> renderImage)
void ImageAsset::renderImage(rcp<RenderImage> renderImage)
{
m_RenderImage = std::move(renderImage);
}
Expand Down
11 changes: 8 additions & 3 deletions src/shapes/image.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,15 @@ using namespace rive;

void Image::draw(Renderer* renderer)
{
rive::RenderImage* renderImage;

rive::ImageAsset* asset = this->imageAsset();
if (asset == nullptr || renderOpacity() == 0.0f ||
(renderImage = asset->renderImage()) == nullptr)
if (asset == nullptr || renderOpacity() == 0.0f)
{
return;
}

rive::RenderImage* renderImage = asset->renderImage();
if (renderImage == nullptr)
{
return;
}
Expand Down
5 changes: 1 addition & 4 deletions utils/no_op_factory.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,4 @@ std::unique_ptr<RenderPaint> NoOpFactory::makeRenderPaint()
return rivestd::make_unique<NoOpRenderPaint>();
}

std::unique_ptr<RenderImage> NoOpFactory::decodeImage(Span<const uint8_t>)
{
return std::unique_ptr<NoOpRenderImage>();
}
rcp<RenderImage> NoOpFactory::decodeImage(Span<const uint8_t>) { return nullptr; }
2 changes: 1 addition & 1 deletion viewer/include/viewer/tess/viewer_sokol_factory.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,6 @@
class ViewerSokolFactory : public rive::SokolFactory
{
public:
std::unique_ptr<rive::RenderImage> decodeImage(rive::Span<const uint8_t>) override;
rive::rcp<rive::RenderImage> decodeImage(rive::Span<const uint8_t>) override;
};
#endif
6 changes: 2 additions & 4 deletions viewer/src/sample_tools/sample_atlas_packer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ class AtlasRenderImage : public RenderImage

class AtlasPackerFactory : public NoOpFactory
{
std::unique_ptr<RenderImage> decodeImage(Span<const uint8_t> bytes) override
rcp<RenderImage> decodeImage(Span<const uint8_t> bytes) override
{
auto bitmap = Bitmap::decode(bytes.data(), bytes.size());
if (bitmap)
Expand All @@ -40,9 +40,7 @@ class AtlasPackerFactory : public NoOpFactory
bitmap->pixelFormat(Bitmap::PixelFormat::RGBA);
}

return rivestd::make_unique<AtlasRenderImage>(bitmap->bytes(),
bitmap->width(),
bitmap->height());
return make_rcp<AtlasRenderImage>(bitmap->bytes(), bitmap->width(), bitmap->height());
}
return nullptr;
}
Expand Down
10 changes: 5 additions & 5 deletions viewer/src/tess/viewer_sokol_factory.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
#include "rive/tess/sokol/sokol_tess_renderer.hpp"
#include "sokol_gfx.h"

std::unique_ptr<rive::RenderImage> ViewerSokolFactory::decodeImage(rive::Span<const uint8_t> bytes)
rive::rcp<rive::RenderImage> ViewerSokolFactory::decodeImage(rive::Span<const uint8_t> bytes)
{
auto bitmap = Bitmap::decode(bytes.data(), bytes.size());
if (bitmap)
Expand All @@ -26,10 +26,10 @@ std::unique_ptr<rive::RenderImage> ViewerSokolFactory::decodeImage(rive::Span<co
new rive::SokolRenderImageResource(bitmap->bytes(), bitmap->width(), bitmap->height()));

static rive::Mat2D identity;
return rivestd::make_unique<rive::SokolRenderImage>(imageGpuResource,
bitmap->width(),
bitmap->height(),
identity);
return rive::make_rcp<rive::SokolRenderImage>(imageGpuResource,
bitmap->width(),
bitmap->height(),
identity);
}
return nullptr;
}
Expand Down
4 changes: 2 additions & 2 deletions viewer/src/viewer_content/image_content.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,10 @@

class ImageContent : public ViewerContent
{
std::unique_ptr<rive::RenderImage> m_image;
rive::rcp<rive::RenderImage> m_image;

public:
ImageContent(std::unique_ptr<rive::RenderImage> image) : m_image(std::move(image)) {}
ImageContent(rive::rcp<rive::RenderImage> image) { m_image = std::move(image); }

void handleDraw(rive::Renderer* renderer, double) override
{
Expand Down

0 comments on commit 04de471

Please sign in to comment.