Render light

[ivovandongen]

Splits Light in Style and Render portions. Also adds accessors for api clients.

[jfirebaugh]

👍

[jfirebaugh]

Unused.

[ivovandongen]

👍

[ivovandongen]

@jfirebaugh Thanks for the review. I made some additional changes though. In c4c040c I've added an observer that keeps the RenderLight up-to-date as discussed in chat. I've opted to update the RenderLight as a whole for now, we could make that more granular. What do you think?

[jfirebaugh]

I think c4c040c923a0269bc96a50c444fd62dac3606fed should be adjusted slightly to follow the pattern established by layers more closely:

• Light owns a Light::Impl (soon to be made immutable)
• RenderLight has a Light::Impl reference (soon to be made immutable)
• On notification of changes, the observer sets an update flag (soon to be replaced with diffing immutable objects)
• When the style updates and sees the flag, it tells the RenderLight to recalculate (this may be dependent on [core] Omnibus Style::update method #8888)

[ivovandongen]

@jfirebaugh I've implemented the Impl, but ran into two hurdles:

• The Light currently is copied in some places, for example from Parser to Style. I didn't want to move there as this leaves the parser in an odd state.
• in Style::setLight we take a unique_ptr and set this as the new light (including a new Impl), this would break the reference in RenderLight as the old Impl is destroyed.

I solved both by using a shared_ptr instead of a unique_ptr in Light and a reference in RenderLayer. This makes Light copyable and ensures that RenderLights handle on the Impl is valid.

Alternatively, we could get rid of the copying in the parser and update Impl::properties in Style::setLight. But as we are moving to a shared_ptr solution anyway, this seems like a waste at this point.

[kkaefer]

I solved both by using a shared_ptr instead of a unique_ptr in Light and a reference in RenderLayer.

My understanding is that we're moving to shared_ptr for the immutable implementation for precisely that reason.

[jfirebaugh]

My understanding is that we're moving to shared_ptr for the immutable implementation for precisely that reason.

Correct, although it will be encapsulated in an Immutable template wrapper. My current design:

#pragma once

#include <memory>

namespace mbgl {

/**
 * `Mutable<T>` is a uniquely owning reference to a `T`. It can be efficiently converted to `Immutable<T>`.
 * 
 * The lifecycle of `Mutable<T>` and `Immutable<T>` is as follows:
 * 
 *   1. Create a `Mutable<T>` using `makeMutable(...)`
 *   2. Mutate it freely
 *   3. When you're ready to freeze its state and enable safe cross-thread sharing, move assign or
 *      move construct it to `Immutable<T>`
 *
 * The reason that `Mutable<T>` exists, rather than simply using a `std::unique_ptr<T>`, is to take advantage
 * of the underlying single-allocation optimization provided by `std::make_shared`.
 */
template <class T>
class Mutable {
public:
    Mutable(Mutable&&) = default;
    Mutable& operator=(Mutable&&) = default;

    Mutable(const Mutable&) = delete;
    Mutable& operator=(const Mutable&) = delete;

    T* get() { return ptr.get(); }
    T* operator->() { return ptr.get(); }
    T& operator*() { return *ptr; }

private:
    Mutable(std::shared_ptr<T>&& s)
        : ptr(std::move(s)) {}

    std::shared_ptr<T> ptr;

    template <class S> friend class Immutable;
    template <class S, class... Args> friend Mutable<S> makeMutable(Args&&...);
};

template <class T, class... Args>
Mutable<T> makeMutable(Args&&... args) {
    return Mutable<T>(std::make_shared<T>(std::forward<Args>(args)...));
}

/**
 * `Immutable<T>` is a shared reference to a T that cannot change. Construction requires
 * a transfer of unique ownership from a `Mutable<T>`; once constructed it has the same behavior
 * as `std::shared_ptr<const T>` but with better indication of intent.
 *
 * Normally one should not share state between threads because it's difficult to verify the
 * absence of read/write data races. `Immutable` provides a guarantee that no writes are
 * possible, and instances therefore can be freely transferred and shared between threads.
 */
template <class T>
class Immutable {
public:
    Immutable() = default;

    template <class S>
    Immutable(Mutable<S>&& s)
        : ptr(std::const_pointer_cast<const S>(std::move(s.ptr))) {}

    template <class S>
    Immutable(Immutable<S>&& s)
        : ptr(std::move(s.ptr)) {}

    template <class S>
    Immutable(const Immutable<S>& s)
        : ptr(s.ptr) {}

    template <class S>
    Immutable& operator=(Mutable<S>&& s) {
        ptr = std::const_pointer_cast<const S>(std::move(s.ptr));
        return *this;
    }

    template <class S>
    Immutable& operator=(Immutable<S>&& s) {
        ptr = std::move(s.ptr);
        return *this;
    }

    template <class S>
    Immutable& operator=(const Immutable<S>& s) {
        ptr = s.ptr;
        return *this;
    }

    const T* get() const { return ptr.get(); }
    const T* operator->() const { return ptr.get(); }
    const T& operator*() const { return *ptr; }

private:
    Immutable(std::shared_ptr<const T>&& s)
        : ptr(std::move(s)) {}

    std::shared_ptr<const T> ptr;

    template <class S> friend class Immutable;
    template <class S> friend class EnableImmutableFromThis;
    template <class S, class U> friend Immutable<S> staticImmutableCast(const Immutable<U>&);
};

template <class T>
class EnableImmutableFromThis : public std::enable_shared_from_this<const T> {
public:
    Immutable<T> immutableFromThis() const {
        return Immutable<T>(this->shared_from_this());
    }
};

template <class S, class U>
Immutable<S> staticImmutableCast(const Immutable<U>& u) {
    return Immutable<S>(std::static_pointer_cast<const S>(u.ptr));
}

} // namespace mbgl

[jfirebaugh]

This isn't used in the header, right? It can move to the .cpp file an the src directory tree.