Does this really need a class with operator overloading?

incides -> indices

It's unclear why this constructor does not initialize add_barycentric_coord_y. If it's intentional, it's unclear what the meaning of such a partially initialized class is.

I'm not sure that storing loop indices will be a good use of memory if they can be looked up somewhere else.

PackedPixelRow may be a better name, I think in software terms "package" has a different meaning.

This only needs 2 floats.

I don't think this typedef is helpful if it's only used in one place.

There should be a comment explaining what this class is. It's not obvious what the meaning of "tile" is in this context, it appears to be UDIM tiles.

I would move BKE_image_partial_update_mark_region out of this function and into the one place that calls it, and rename this to clear_dirty. And then add a mark_dirty that handles part of this logic:

if (pixels_painted) {
  int2 start_image_coord(encoded_pixels.start_image_coordinate.x,
                         encoded_pixels.start_image_coordinate.y);
  BLI_rcti_do_minmax_v(&tile_data->dirty_region, start_image_coord);
  BLI_rcti_do_minmax_v(&tile_data->dirty_region,
                       start_image_coord + int2(encoded_pixels.num_pixels + 1, 0));
  tile_data->flags.dirty = true;
}

That way there are methods that clearly affect the data that is contained in this class.

This is unused.

This is not implemented.

It's not clear to me how an ::extractor sub namespace helps.

Abstractions like this method I also find confusing. It's not obvious what the return of a method name tag_visited is (successfully tagged?), so in order to understand the one function that uses this return value you have to jump around the code, while it would have been simpler to just inline this code.

Would not use doxygen unless you do it consistently.

I would not use using here, makes it hard to tell what data structure comes from where.

Is this only extending along the x-axis (why?), or just named confusingly?

This is not used.

This is the kind of abstraction that confuses me, the contents of this class is not what would usually be described as a pixel.

Would not use goto, has a specific meaning that's not this. Just next_pixel can work.

store -> write for consistency with read.

Is there a plan to abstract this kind of thing and share code with vertex colors? Or would there be separate implementations of brushes for vertex colors and image textures?

I'm not sure how much code it would reduce overall though, if any.

For consistency, this typename could be ImageBuffer or the above class could be ImagePixelAccessorByte4.

Would consistently used either "packed" or "encoded", not a combination of both.

This kind of update check could be moved outside the inner loop, into a separate function.

It's not clear to me this works? It's possible for all 3 vertices to fall outside the brush radius but part of the triangle to still be within the brush radius.

You'd need to compute the bounding box of the 3 vertices and then have a test for that against the brush radius?

Locking is currently only used for render results and viewers, so no. If a painted image needed locking it would need to be moved outside this function so multithreading keeps working.

It seems more natural to loop over the tiles in the node and then find the image tile from that?

If performance is a concern there could be an cached array for mapping, but I don't think that's the case, and it's already doing a loop over the tile data anyway.

I would not use a function call here, you're already editing the tile data directly, might as well set the node dirty flag directly. Otherwise it seems like this function call may be used to mark the entire node dirty while that would really require more steps.

It might be a good idea to interface with C++ a bit differently. One thing I've done is putting stuff in the #ifdef _cplusplus:

#ifdef __cplusplus

namespace blender::pbvh::NodeData {
  struct NodeData;
}
using blender::pbvh::pixels::NodeData;

extern "C" {
#else 
struct NodeData;
#endif

This of course request the usage of struct, which is default-public. But that's what public/private keywords are for.

Is it necessary to make a subclass here?

I think it's better to use the tessellation that's stored inside the nodes directly. You can use my patch for that, or just iterate over the triangles directly; see the doc comments for PBVHNode.prim_indices in master (which I've clarified a bit).

It might be a good idea to make isect_seg_seg_v2_simple inline.

Btw don't use closest_on_tri_to_point_v3 (if you were thinking about it). It's a pretty expensive, it's used by dyntopo and is usually the most expensive operation in profiling tests.

I will remove this optimization from this patch, eventually this should happen per primitive to improve performance. Doesn't have to be part of the initial brush.

I didn't known of other areas that use this. For this case it would only remove a few casts. The downside is that you loose the FQN in C what could lead to misunderstanding and naming collisions. Personally I would hide it in a void* to reduce confusion.

I want to replace this with an area coverage check. inlining this would IMO not help that much, there are more effective ways to optimize the branch prediction.

Have replaced it with last_paint_canvas_key

After testing it seems that the performance drops back more than expected. Will add a primitive test.

I added T97344 to keep track of this. Especially with the UV seam extraction I don't see a good way to extract the data on a per node.

Inside the brush colors are stored in sRGB. Other areas convert it to scene linear when needed, mostly when working with float color attributes (geometry) and float textures.
Perhaps on short notice we coult convert it to scene linear via srgb_to_linearrgb_v4 and then apply OCIO processor. and add a note above it.

The brush color is converted once per step, I didn't detect any performance issue so far.